Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[MRESOLVER-408] Make master 2.0.0-SNAPSHOT #335
[MRESOLVER-408] Make master 2.0.0-SNAPSHOT #335
Changes from 6 commits
aaefaf1
6f974f9
0c88f1d
981ad85
14fae66
cf21738
1fec038
217b8c1
16c8d8a
6b9d179
9bc11e6
05d3aa2
7cf2a66
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the benefit to require Java 17 to build it? Why is 11 not enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO next to last LTS is good compromise for build time requirement for our anyway dated stack we use. In other words, I want to encourage experiments, and am willing even to go for Java 21 but there are some spotless issues (@slawekjaranowski plz help). Also, I must emphasize: most resolver modules will be still release=8 compiled, but probably not all. As we may have new modules, that do want all the new nice things we can get.
Am pretty much sure, that our users run Maven on latest Java versions (unlike "aligners"), so as resolver being one of the Maven fundamentals libraries, let's make a leap here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue with spotless fix by workaround in parent apache/maven-parent#135
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still don't see a reason to artificially limit this. You still can build with newer ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is limit only for minimum - you can build by newer versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There ARE technical (and human) justifications. For example, remember this thread? 😄
https://lists.apache.org/thread/k5sc3w7w4xkgjfzyxxltg06722dvspk7
In short, we tend to say "Java 8" but there are important Java updates, hence there is java 8u172 from 2018 and Java 8u371 from 2023. This thread happened to us, and clearly shows there is sense to set "lower limit". I just dont want to waste my time and resources on things like happened in that thread anymore.
This limit simply says (and means) "recent Java 17" and NOT "just any java 17" as we used to say so far. This is far better, explicit, and even the error message makes sense as shown here:
https://lists.apache.org/thread/tz4qfqhvrvf2fr84l8loqsfpmcf5poz4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can enforce 8u300+, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can, but why would I? I want to add modules that do HTTP/2, and require Java11+, moreover, for testing those I may want to use dependencies (test scoped) that are Java17...
What would limiting build time Java requirement to 8u300 help with those above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have Java 17 test deps already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imho, I would keep it as low as needed, but certainly would not put any barrier in raising the build time requirement. This would allow using multi-release JARs if needed or even leverage new JEP such as FFM (for accessing native code or memory). Whatever is needed at build time should be fine.