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

Also detect @jakarta.inject.Inject annotations #377

Merged
merged 2 commits into from
Jan 27, 2025

Conversation

mkurz
Copy link
Contributor

@mkurz mkurz commented Jan 26, 2025

Currently, you are only looking for javax.inject.Inject annotations. However, in Play, we are switching to Guice 7, which eventually only supports jakarta.inject: https://github.com/google/guice/wiki/Guice700#jee-jakarta-transition

Our samples repo contains a project that is using macwire, but now that we drop javax.inject, that project does not work anymore: https://github.com/playframework/play-samples/tree/main/scala/macwire-di (actually this is the PR that makes it fail: playframework/play-samples#799 - which will be updated to just disable that sample for now until this PR here makes it into your next release).

Looking at your code, all it needs is that the isInjectAnnotation method also considers the jakarta annotion.

I can confirm, after testing locally, this fix makes the sample's tests work again, on both Scala 2 and 3.

Would be great if you could cut a new release with this fix. Thanks!

@mkurz
Copy link
Contributor Author

mkurz commented Jan 27, 2025

ahh.. need to update tests... ;)

@adamw adamw merged commit 07293d6 into softwaremill:master Jan 27, 2025
6 checks passed
@mkurz mkurz deleted the jakarta.inject branch January 27, 2025 08:10
@mkurz
Copy link
Contributor Author

mkurz commented Jan 27, 2025

Thanks!
@adamw Can you cut 2.6.6?

mkurz added a commit to mkurz/play-samples that referenced this pull request Jan 27, 2025
@adamw
Copy link
Member

adamw commented Jan 27, 2025

In progress :)

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