Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update/sponge 8 fix #421

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

Intybyte
Copy link
Contributor

@Intybyte Intybyte commented Mar 4, 2025

This fix allows compatibility for sponge8 till sponge 10 (tested at least) and is more flexible towards sponge api breakages.

Requires testing (will test in a few days or when am free, or someone else ig).

@chickeneer
Copy link
Collaborator

I am concerned about the removal of the removal of constructors for SpongeCommandSource. Admittedly, I have no way to know if anyone is or isn't using it. But if I simply merge and deploy this to the current version of ACF - it has the technical potential to break if for some reason someone had called that API.

I am aware that within the ACF module - it doesn't have an affect, but would prefer to find a solution which keeps constructors where possible with a Deprecated or something.

@chickeneer
Copy link
Collaborator

I should say. I am not necessarily opposed to merging it as proposed. Just want the before mentioned risk be part of the conversation.

@Intybyte Intybyte marked this pull request as ready for review March 24, 2025 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants