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

Replace JavaX Injection with Google's Injection #276

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

R00tB33rMan
Copy link

No description provided.

@A248
Copy link
Owner

A248 commented Aug 9, 2024

I know why you made this PR, and I appreciate that as a maintainer of Velocity-CTD you care about having other software run on it.

Still, though, I really have to ask: why did you have to break compatibility? It's not like Guice 7.0 offers more features or is more performant than Guice 6.0. The difference is minimal and limited exactly to javax.inject/jakarta.inject support. By "upgrading" the dependency, however, you broke plugins using javax.inject, including existing plugin jars that have already been released.

If I merge this PR and call the problem solved, I have to make another release. Because if I don't, I know I'm still going to get people who use Velocity-CTD with the latest release jar (1.0.0-RC-2).

@R00tB33rMan
Copy link
Author

I know why you made this PR, and I appreciate that as a maintainer of Velocity-CTD you care about having other software run on it.

Still, though, I really have to ask: why did you have to break compatibility? It's not like Guice 7.0 offers more features or is more performant than Guice 6.0. The difference is minimal and limited exactly to javax.inject/jakarta.inject support. By "upgrading" the dependency, however, you broke plugins using javax.inject, including existing plugin jars that have already been released.

If I merge this PR and call the problem solved, I have to make another release. Because if I don't, I know I'm still going to get people who use Velocity-CTD with the latest release jar (1.0.0-RC-2).

Of course! I've received a few complaints; however, I feel as though it is important to keep things maintained and up-to-date. Thankfully, Google's Injections are compatible with 7.0.0 and 6.0.0. There may come a time in the near future when you will be required to make this change (such as the possibility of Guice receiving yet another upgrade that contains more important changes). Having this issue effectively knocked out and dealt with ensures that users won't complain about this issue in the future.

@A248
Copy link
Owner

A248 commented Aug 10, 2024

Guice 6.0 is maintained and up-to-date. Guice 7.0 is a breaking change and LibertyBans targets Velocity 3.x. If there is a breaking change in Velocity to use a new Guice version, we will update.

Otherwise, for the reason I mentioned already (my concern you haven't allayed), I'm going to ask you to fix the breaking change in Velocity-CTD. This is your responsibility after all, and I don't have the time nor resources to make an official release right now with a new plugin jar.

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