search

A248 / LibertyBans

The be-all, end-all of discipline.
https://ci.hahota.net:8443/job/LibertyBans/
GNU Affero General Public License v3.0
169 stars 41 forks source link

Replace JavaX Injection with Google's Injection #276

Open R00tB33rMan opened 3 months ago

A248 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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.