-
Notifications
You must be signed in to change notification settings - Fork 7
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
[Build] Fix build plugin version warnings and build with Maven 3.9.9 #211
base: master
Are you sure you want to change the base?
Conversation
Github does not consider the changes in workflows in verification builds of PRs created by non-committers, so the verification does not test this. |
.github/workflows/buildAndTest.yml
Outdated
- name: Set up Maven | ||
uses: stCarolas/setup-maven@d6af6abeda15e98926a57b5aa970a96bb37f97d1 # v5 | ||
with: | ||
maven-version: 3.9.9 |
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 would like to update the Maven version via using Java 21 in the setup-java task as the default build Java version transitively instead of introducing a new third-party GH action dependency here. Is there any reason to switch from the Maven version that is shipped with setup-java right now to justify doing this change?
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.
As far as I can tell, the Maven version installed by default it defined by the runner-image:
https://github.com/actions/runner-images/blob/ubuntu22/20241015.1/images/ubuntu/Ubuntu2204-Readme.md
I'm not aware that the setup-java action configures the Maven version, although it provides a Maven-cache and support for the maven tool-chain.
Currently an update of Maven is not necessary, but it might become necessary if you update for example Tycho (I'm not exactly sure about the requirements of the latest Tycho release.
But of course Maven could also just be updated explicitly if it's necessary.
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.
Looks like #219 passed without an update so the current default is fine.
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.
So what about the remaining changes in this PR? Were they warnings as well?
4170d00
to
e038045
Compare
In the most recent versions of Maven these warnings are now errors
e038045
to
7be30f5
Compare
In the most recent versions of Maven these warnings are now errors