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

Make Generic Provides Changes Backwards Compatible #566

Closed
wants to merge 4 commits into from

Conversation

SentryMan
Copy link
Collaborator

@SentryMan SentryMan commented May 13, 2024

Upon experimentation, it seems the Class<?> to Type changes we've made are backwards incompatible. Breaking every single multi-module project/plugin library based on avaje is pretty cringe so this PR makes everything (mostly) backwards compatible.

  • Reverts changes to Module/Plugin classes and adds deprecation notices
  • Creates super classes based on Type for the sake of compatibility (for whatever reason this works)
  • renames PropertyRequiresPlugin
  • modifies scope builder, generator, and maven plugin to service load the new interfaces

Requires avaje/avaje-spi-service#18 and also will eventually care of #559

@SentryMan SentryMan added the enhancement New feature or request label May 13, 2024
@SentryMan SentryMan requested a review from rbygrave May 13, 2024 21:53
@SentryMan SentryMan self-assigned this May 13, 2024
@SentryMan SentryMan added this to the 10.0 milestone May 13, 2024
@rob-bygrave
Copy link
Contributor

IMO you gotta stop including multiple changes into a single PR like this. It generally is a pain to review and becomes an all-or-nothing merge and reduces the ability to revert individual changes etc in the future.

In addition imo you have to try and honour the existing code format. Making format changes only for the sake of code format changes increases the diff and makes more work for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants