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

Manage maven version similarly as the other dependency versions are m… #9353

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

gzsombor
Copy link
Member

…anaged

@@ -18,7 +18,7 @@
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>

<maven.version>3.9.5</maven.version>
<maven.version>{{mavenVersion}}</maven.version>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gzsombor @pascalgrimaud @murdos If maven.version is used to enforce minimum maven version with maven-enforcer-plugin (like in the generator-jhipster), this should be version 3.2.5.

See https://maven.apache.org/docs/history.html:
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DanielFran I agree. We previously had a similar discussion: #8933 (comment)
I however think that requiring 3.2.5 is way too low. Recent (3.8+) maven versions contains a lot of improvements and it would be a shame to not raise the bar a little.
If we require 3.2.5 version, then the enforce verification doesn't have much value.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'm going to rename the mavenVersion to 'latestMavenVersion' or something like that, and keep 'enforcer-plugin' as is

@@ -45,6 +45,7 @@
<archunit-junit5.version>1.2.1</archunit-junit5.version>
<cucumber.version>7.16.1</cucumber.version>
<!-- Maven Plugins -->
<maven.version>3.9.6</maven.version>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maven wrapper version is already managed by renovate that automatically update it, so I don't really see the added value with theses changes:
image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My main goal is to have a way to get the current maven/gradle version from a module - as I try to create a jhipster module to create Mise configuration, so I could have a minimal, declarative project setup.
Any alternative idea, how and where can I get those values ? I would rather not duplicate that constant in that module.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main issue is that by doing that you're breaking automatic update of maven wrapper.
The source of truth for maven wrapper version in jhipster-lite is src/main/resources/generator/buildtool/maven/.mvn/wrapper/maven-wrapper.properties since it's handled by our automatic dependencies updater (renovate).

If for your specific needs you need to get the maven wrapper version used by jhipster-lite, you could implement a reader, in your own module, that will extract the version from src/main/resources/generator/buildtool/maven/.mvn/wrapper/maven-wrapper.properties:
mise/domain/MavenWrapperVersionReader.java

interface MavenWrapperVersionReader {
  Version readVersion();
}

implemented by
mise/infractructure/secondary/FilesystemMavenWrapperVersionReader.java

@Component
class FilesystemMavenWrapperVersionReader implements MavenWrapperVersionReader {
  @Override
  public Version readVersion() {
  // load generator/buildtool/maven/.mvn/wrapper/maven-wrapper.properties content from classpath
  // using a regexp to extract maven version
  }
}

@@ -18,7 +18,7 @@
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>

<maven.version>3.9.5</maven.version>
<maven.version>{{mavenVersion}}</maven.version>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DanielFran I agree. We previously had a similar discussion: #8933 (comment)
I however think that requiring 3.2.5 is way too low. Recent (3.8+) maven versions contains a lot of improvements and it would be a shame to not raise the bar a little.
If we require 3.2.5 version, then the enforce verification doesn't have much value.

Copy link

codecov bot commented Mar 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (055ea4f) to head (9b1a87e).
Report is 337 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##                main     #9353   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity      2937      2938    +1     
===========================================
  Files            734       734           
  Lines          12756     12762    +6     
  Branches         257       257           
===========================================
+ Hits           12756     12762    +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@murdos murdos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As stated in this comment this is breaking automatic maven wrapper updates with renovate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants