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

solves #62: Remove all dependencies on Guava #63

Merged
merged 4 commits into from
Aug 7, 2024

Conversation

royteeuwen
Copy link
Contributor

No description provided.

@royteeuwen royteeuwen linked an issue Aug 6, 2024 that may be closed by this pull request
@royteeuwen royteeuwen self-assigned this Aug 6, 2024
@royteeuwen royteeuwen force-pushed the 62-get-rid-of-google-guava-dependency branch from a0c17ab to fc13cff Compare August 6, 2024 15:25
@royteeuwen royteeuwen force-pushed the 62-get-rid-of-google-guava-dependency branch from fc13cff to 1211f82 Compare August 6, 2024 15:36
@royteeuwen
Copy link
Contributor Author

@kwin here is a PR. Can you please validate it from your side :)?

@kwin
Copy link
Contributor

kwin commented Aug 6, 2024

You should definitely use https://maven.apache.org/plugins/maven-compiler-plugin/examples/set-compiler-release.html to check against the Java API you claim to be compatiblewith.

@royteeuwen
Copy link
Contributor Author

royteeuwen commented Aug 6, 2024

You should definitely use https://maven.apache.org/plugins/maven-compiler-plugin/examples/set-compiler-release.html to check against the Java API you claim to be compatible with.

@kwin I spent an hour trying to get it to work, but didn't get it working. Probably the actual compiler (groovy-eclipse-compiler) does not take the release property correctly into account :(.

@royteeuwen
Copy link
Contributor Author

Once AEM goes for a Java 17 / 21 release, I'll see if we will drop Java 8. No requirements at this moment to do so, seeing as we can use all Groovy syntax additions to add most things we want

@royteeuwen royteeuwen requested a review from kwin August 6, 2024 19:30
@kwin
Copy link
Contributor

kwin commented Aug 7, 2024

@kwin I spent an hour trying to get it to work, but didn't get it working. Probably the actual compiler (groovy-eclipse-compiler) does not take the release property correctly into account :(.

You can use https://www.mojohaus.org/animal-sniffer/animal-sniffer-maven-plugin/index.html instead as that works on the compiled classes (i.e. independent which compiler generated those)

@royteeuwen
Copy link
Contributor Author

The animal sniffer plugin also doesn't seem to work properly with the code generated by Groovy either. Going to merge this PR as is for now

@royteeuwen royteeuwen merged commit beb948e into main Aug 7, 2024
4 checks passed
@royteeuwen royteeuwen deleted the 62-get-rid-of-google-guava-dependency branch August 7, 2024 14:27
Copy link

sonarqubecloud bot commented Aug 7, 2024

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.

Get rid of Google Guava dependency
2 participants