[MPOM-289] Use properties to define the versions of plugins #155

slawekjaranowski · 2023-05-23T22:16:06Z

discussion invite sent to release-discuss@apache.org: https://lists.apache.org/list?release-discuss@apache.org:2023-6

ctubbsii · 2023-05-24T23:27:00Z

pom.xml

+    <apacheRatPluginVersion>0.15</apacheRatPluginVersion>
+    <mavenAntrunPluginVersion>3.1.0</mavenAntrunPluginVersion>
+    <mavenAssemblyPluginVersion>3.6.0</mavenAssemblyPluginVersion>
+    <mavenCleanPluginVersion>3.2.0</mavenCleanPluginVersion>
+    <mavenCompilerPluginVersion>3.11.0</mavenCompilerPluginVersion>
+    <mavenDependencyPluginVersion>3.6.0</mavenDependencyPluginVersion>
+    <mavenDeployPluginVersion>3.1.1</mavenDeployPluginVersion>
+    <mavenEarPluginVersion>3.3.0</mavenEarPluginVersion>
+    <mavenEnforcerPluginVersion>3.3.0</mavenEnforcerPluginVersion>
+    <mavenGpgPluginVersion>3.1.0</mavenGpgPluginVersion>
+    <mavenHelpPluginVersion>3.4.0</mavenHelpPluginVersion>
+    <mavenInstallPluginVersion>3.1.1</mavenInstallPluginVersion>
+    <mavenInvokerPluginVersion>3.5.1</mavenInvokerPluginVersion>
+    <mavenJarPluginVersion>3.3.0</mavenJarPluginVersion>
+    <mavenJavadocPluginVersion>3.5.0</mavenJavadocPluginVersion>
+    <mavenProjectInfoReportsPluginVersion>3.4.3</mavenProjectInfoReportsPluginVersion>
+    <mavenReleasePluginVersion>3.0.0</mavenReleasePluginVersion>
+    <mavenRemoteResourcesPluginVersion>3.1.0</mavenRemoteResourcesPluginVersion>
+    <mavenResourcesPluginVersion>3.3.1</mavenResourcesPluginVersion>
+    <mavenScmPluginVersion>2.0.1</mavenScmPluginVersion>
+    <mavenScmPublishPluginVersion>3.2.1</mavenScmPublishPluginVersion>
+    <mavenShadePluginVersion>3.4.1</mavenShadePluginVersion>
+    <mavenSitePluginVersion>3.12.1</mavenSitePluginVersion>
+    <mavenSourcePluginVersion>3.3.0</mavenSourcePluginVersion>
+    <mavenWarPluginVersion>3.3.2</mavenWarPluginVersion>


The idea is nice, but can we use a naming convention that is more sortable, and groups version-related properties apart from other properties, and uses the artifact name as part of the property, so it's easy to grep for these? Something like maven-release-plugin -> version.maven-release-plugin.

I want to reuse those values in many place also in documentation in .vm files ... but we can not use 'dot' for velocity templates.

@slawekjaranowski The maven-site-plugin documentation says you can use $context.get("my.property"), so that's not a problem.

@ctubbsii I know such workarounds for sitedocs ... but I would like to use properties as simply as possible without additional hacks.
So I will merge as is - if no more objections.

I stand by my objection that I think the naming convention could be better... even if you choose not to use dots, using the same name as the plugin artifact and using the word "version" as the prefix rather than the suffix, is better for quickly finding these. It wasn't a problem when there were only a few, but as you're making a bunch of them, keeping them organized becomes more important.

I still think the workaround for the dot in the velocity templates is not a problem... I don't think this should cater to that edge case, because dots are common in Maven and Java, even if velocity doesn't like them. But, even if you do cater to that edge case, there's plenty of ways to improve the naming scheme without using dots.

+1 for dots

Ok, I see that more of us is for using dots, and we have also idea for prefix.

So what do you think about:
version.<artifactId> - like: version.maven-release-plugin - thanks @ctubbsii 😄

perso I usually use <artifactId>.version such <maven-release-plugin.version>
it looks more similar to object.property

perso I usually use <artifactId>.version such <maven-release-plugin.version>
it looks more similar to object.property

+1

What about the old properties ?

<surefire.version>3.1.0</surefire.version> <maven.plugin.tools.version>3.9.0</maven.plugin.tools.version>

Are you going to change them to be consistent with the new ones ? Any concerns about backward compatibility ?

I would suggest keeping any existing ones, and only following a consistent convention for new ones, for now. Changing the existing ones to follow the same convention (with a possible attempt for backwards compatibility, through the use of property referencing another property, or through a profile activated using the legacy property, or something else), could be done in a subsequent PR, if somebody wanted to do it. For now, it would be enough to just establish the convention using new properties.

olamy · 2023-05-24T23:33:15Z

I like the idea. Then it will be possible to use interpolation in it pom files to use same version and avoid some download.

gnodet

Looks good, I don't have any particular opinion about the naming...

michael-o

Very nice!

hgschmie · 2023-06-02T17:57:35Z

I use dep.plugin.<plugin-name>.version properties for a very long time: https://github.com/basepom/basepom/blob/main/poms/foundation/pom.xml#L236-L243

Using a hierarchical naming scheme gives you an opportunity to sort those (e.g. with the sortpom plugin - https://github.com/Ekryd/sortpom) and not have the versions strewn all over the place (the names all align on dep.plugin first).

hboutemy · 2023-06-03T09:23:03Z

Oh, one other comment on this: I've noticed that overriding versions in the child POMs by overriding the property does not cause versions-maven-plugin to notice newer versions when running: mvn -U versions:display-plugin-updates. That's not really an issue for this project... more a deficiency in versions-maven-plugin... but I wanted to mention it here, because if a user used these new properties to override the plugin versions instead of defining a version override in their own <pluginManagement> section, they might not notice that the versions-maven-plugin has that limitation.

is there a GH issue opened about that?

slawekjaranowski · 2023-06-05T22:47:45Z

Oh, one other comment on this: I've noticed that overriding versions in the child POMs by overriding the property does not cause versions-maven-plugin to notice newer versions when running: mvn -U versions:display-plugin-updates. That's not really an issue for this project... more a deficiency in versions-maven-plugin... but I wanted to mention it here, because if a user used these new properties to override the plugin versions instead of defining a version override in their own <pluginManagement> section, they might not notice that the versions-maven-plugin has that limitation.

You can use: mvn versions:display-property-updates in such case

ctubbsii · 2023-06-06T04:00:15Z

You can use: mvn versions:display-property-updates in such case

Thanks @slawekjaranowski ! That's good to know.

bmarwell · 2023-06-06T06:34:16Z

pom.xml

    <surefire.version>3.1.0</surefire.version><!-- for surefire, failsafe and surefire-report -->
    <maven.plugin.tools.version>3.9.0</maven.plugin.tools.version><!-- for m-plugin-p and maven-plugin-annotations -->
    <assembly.tarLongFileMode>posix</assembly.tarLongFileMode>
+
+    <!-- plugins versions -->
+    <apacheRatPluginVersion>0.15</apacheRatPluginVersion>
+    <mavenAntrunPluginVersion>3.1.0</mavenAntrunPluginVersion>
+    <mavenAssemblyPluginVersion>3.6.0</mavenAssemblyPluginVersion>


Please stay consistent in Naming: surefire.version != mavenAssemblyPluginVersion.

If velocity has problems, let's fix velocity. In freemarker we could just use another syntax, I expect velocity to have the same ability. 3rd party tools (ie non-maven) should not have impact on naming.

michael-o · 2023-06-06T16:29:35Z

We can have endless debates about a format, at the end you cannot please everyone. Let's stick to the one we have, it is established and move on.

martin-g · 2023-06-06T18:26:27Z

Let's stick to the one we have, it is established and move on.

Which one exactly ?

    <surefire.version>3.1.0</surefire.version>       ONE
    <maven.plugin.tools.version>3.9.0</maven.plugin.tools.version> TWO
    <apacheRatPluginVersion>0.15</apacheRatPluginVersion> THREE

michael-o · 2023-06-06T18:38:48Z

Let's stick to the one we have, it is established and move on.

Which one exactly ?

    <surefire.version>3.1.0</surefire.version>       ONE
    <maven.plugin.tools.version>3.9.0</maven.plugin.tools.version> TWO
    <apacheRatPluginVersion>0.15</apacheRatPluginVersion> THREE

The first two have been superseded and lead to problems, e.g., release.version, try and watch it fail.

martin-g · 2023-06-06T18:56:36Z

FWIW I vote for apache-rat-plugin.version
It is much easier to copy/paste the artifactId and append/prepend version.

Alternate implementation to PR apache#155 to demonstrate my alternative proposal with the naming convention of `version.artifactId` for properties. The properties are sorted in the properties section. This also includes individual properties for the plugins that use `${maven.plugin.tools.version}` and `${surefire.version}`, so they can be set individually, but will default to using the current properties that set the version for multiple plugins at once. This also includes MPOM-407 (PR apache#157) to bump the version of maven-release-plugin, as well as a bugfix version bump for surefire/failsafe and maven-project-info-reports-plugin This also includes a bump of the minimum Maven version, because the current version of maven-help-plugin declares a prerequisite Maven 3.6.3. This also fixes the M2Eclipse profile, `only-eclipse`, to remove the upper bound on the version for maven-remote-resources-plugin, because the version specified in this POM has already exceeded the that upper bound, and the profile would not have been working correctly.

ctubbsii · 2023-06-06T21:03:17Z

I created an alternate implementation for my ideas from this discussion at #158, that adopts the version.$artifactId naming convention, and addresses the issue with the existing version properties, as well as fixes a few other tiny things (details in the description of that PR).

Alternate implementation to PR apache#155 to demonstrate my alternative proposal with the naming convention of `version.artifactId` for properties. The properties are sorted in the properties section. This also includes individual properties for the plugins that use `${maven.plugin.tools.version}` and `${surefire.version}`, so they can be set individually, but will default to using the current properties that set the version for multiple plugins at once. This also fixes the M2Eclipse profile, `only-eclipse`, to remove the upper bound on the version for maven-remote-resources-plugin, because the version specified in this POM has already exceeded the that upper bound, and the profile would not have been working correctly.

slawekjaranowski · 2023-06-11T09:32:01Z

suppressed by #158

[MPOM-289] Use properties to define the versions of plugins

ce513ec

slachiewicz approved these changes May 23, 2023

View reviewed changes

ctubbsii reviewed May 24, 2023

View reviewed changes

slachiewicz approved these changes Jun 1, 2023

View reviewed changes

gnodet approved these changes Jun 2, 2023

View reviewed changes

michael-o approved these changes Jun 2, 2023

View reviewed changes

This comment was marked as resolved.

Sign in to view

bmarwell requested changes Jun 6, 2023

View reviewed changes

ctubbsii mentioned this pull request Jun 6, 2023

[MPOM-289] Use properties for all plugin versions (alternate) #158

Merged

gnodet self-requested a review June 9, 2023 18:09

slawekjaranowski closed this Jun 11, 2023

slawekjaranowski deleted the MPOM-289 branch June 11, 2023 13:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MPOM-289] Use properties to define the versions of plugins #155

[MPOM-289] Use properties to define the versions of plugins #155

slawekjaranowski commented May 23, 2023 •

edited by hboutemy

ctubbsii May 24, 2023

slawekjaranowski May 25, 2023

ctubbsii May 25, 2023

slawekjaranowski Jun 1, 2023

ctubbsii Jun 2, 2023

bmarwell Jun 3, 2023

slawekjaranowski Jun 5, 2023

olamy Jun 5, 2023

martin-g Jun 6, 2023

ctubbsii Jun 6, 2023

olamy commented May 24, 2023

gnodet left a comment

michael-o left a comment

This comment was marked as resolved.

hgschmie commented Jun 2, 2023

hboutemy commented Jun 3, 2023

slawekjaranowski commented Jun 5, 2023

ctubbsii commented Jun 6, 2023

bmarwell Jun 6, 2023

michael-o commented Jun 6, 2023

martin-g commented Jun 6, 2023

michael-o commented Jun 6, 2023

martin-g commented Jun 6, 2023

ctubbsii commented Jun 6, 2023

slawekjaranowski commented Jun 11, 2023

[MPOM-289] Use properties to define the versions of plugins #155

[MPOM-289] Use properties to define the versions of plugins #155

Conversation

slawekjaranowski commented May 23, 2023 • edited by hboutemy

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olamy commented May 24, 2023

gnodet left a comment

Choose a reason for hiding this comment

michael-o left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

hgschmie commented Jun 2, 2023

hboutemy commented Jun 3, 2023

slawekjaranowski commented Jun 5, 2023

ctubbsii commented Jun 6, 2023

Choose a reason for hiding this comment

michael-o commented Jun 6, 2023

martin-g commented Jun 6, 2023

michael-o commented Jun 6, 2023

martin-g commented Jun 6, 2023

ctubbsii commented Jun 6, 2023

slawekjaranowski commented Jun 11, 2023

slawekjaranowski commented May 23, 2023 •

edited by hboutemy