You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/01/19 07:37:00 UTC
[GitHub] [maven-apache-parent] kwin opened a new pull request #63: [MPOM-288] - Update m-plugin-p to 3.6.4
kwin opened a new pull request #63:
URL: https://github.com/apache/maven-apache-parent/pull/63
Manage version of maven-plugin-annotations as well
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-apache-parent] ctubbsii commented on a change in pull request #63: [MPOM-288] - Update m-plugin-p to 3.6.4
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #63:
URL: https://github.com/apache/maven-apache-parent/pull/63#discussion_r788113195
##########
File path: pom.xml
##########
@@ -92,10 +92,21 @@ under the License.
<maven.compiler.source>${maven.compiler.target}</maven.compiler.source>
<maven.compiler.target>1.7</maven.compiler.target>
<surefire.version>2.22.2</surefire.version><!-- for surefire, failsafe and surefire-report -->
+ <maven.plugin.tools.version>3.6.4</maven.plugin.tools.version><!-- for m-plugin-p and maven-plugin-annotations -->
<assembly.tarLongFileMode>posix</assembly.tarLongFileMode>
<project.build.outputTimestamp>2021-07-14T15:10:38Z</project.build.outputTimestamp>
</properties>
+ <dependencyManagement>
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.maven.plugin-tools</groupId>
+ <artifactId>maven-plugin-annotations</artifactId>
+ <version>${maven.plugin.tools.version}</version>
+ </dependency>
Review comment:
If this was in the regular `<dependencies>`, I would agree with those users above who suggested adding the `provided` scope. However, this is in the `<dependencyManagement>` section. Adding the scope there would override the default `compile` scope that most people omit when they actually *use* a dependency in the regular `<dependencies>` section in their own POM. Changing that behavior so it's `provided` instead of `compile` can be very confusing to users, and to avoid that confusion, they will redundantly specify the scope themselves anyway. So, placing it here can create confusion, and doesn't really offer much benefit.
So, I don't think the parent POM should manage the scope for the users in a `<dependencyManagement>` section. Instead, the scope should be omitted here, as it currently is, so it defaults to `compile` like users expect when they omit the scope. If they want to mark it `provided`, or any other scope, they should specify the scope themselves when they declare the dependency in their regular `<dependencies>` section of their own POM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-apache-parent] slawekjaranowski commented on a change in pull request #63: [MPOM-288] - Update m-plugin-p to 3.6.4
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on a change in pull request #63:
URL: https://github.com/apache/maven-apache-parent/pull/63#discussion_r788107366
##########
File path: pom.xml
##########
@@ -92,10 +92,21 @@ under the License.
<maven.compiler.source>${maven.compiler.target}</maven.compiler.source>
<maven.compiler.target>1.7</maven.compiler.target>
<surefire.version>2.22.2</surefire.version><!-- for surefire, failsafe and surefire-report -->
+ <maven.plugin.tools.version>3.6.4</maven.plugin.tools.version><!-- for m-plugin-p and maven-plugin-annotations -->
<assembly.tarLongFileMode>posix</assembly.tarLongFileMode>
<project.build.outputTimestamp>2021-07-14T15:10:38Z</project.build.outputTimestamp>
</properties>
+ <dependencyManagement>
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.maven.plugin-tools</groupId>
+ <artifactId>maven-plugin-annotations</artifactId>
+ <version>${maven.plugin.tools.version}</version>
+ </dependency>
Review comment:
should be `provided`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-apache-parent] Tibor17 commented on pull request #63: [MPOM-288] - Update m-plugin-p to 3.6.4
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #63:
URL: https://github.com/apache/maven-apache-parent/pull/63#issuecomment-1021215526
The scope should not be in MPOM.
The MPOM has certain purpose to help and not to break via restrictions.
Simply dependency management declares versions, the same with plugin management.
Plugins section is different story.
The way that we declare versions of dependencies has certain purpose - we prove that the JDK version and Maven API/versions and plugins development would be in consistent state. It's the developer's responsibility to say what dependency would be on certain type of classpath (compilation, test, runtime). The more restrictions of scope we provide in MPOM the more investigation time would be spent on finding out this root cause in MPOM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-apache-parent] ctubbsii commented on pull request #63: [MPOM-288] - Update m-plugin-p to 3.6.4
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #63:
URL: https://github.com/apache/maven-apache-parent/pull/63#issuecomment-1021154511
@slawekjaranowski wrote:
> Currently we have also defined dependency management for it in maven-parent with provided scope
>
> https://github.com/apache/maven-parent/blob/master/pom.xml#L965-L970
And, just as I said, people can't trust the convention breakage, so they override it anyway, making it redundant. Here's two examples:
* https://github.com/apache/maven-compiler-plugin/blob/master/pom.xml#L103
* https://github.com/apache/maven-clean-plugin/blob/master/pom.xml#L92
While I wouldn't recommend breaking conventions in `maven-plugins` parent POM either, it is at least limited to the Apache Maven plugin developers in that POM. If its scope were managed in this POM, it would break conventions for much wider set of projects across the ASF, and even projects outside the ASF.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-apache-parent] slawekjaranowski commented on pull request #63: [MPOM-288] - Update m-plugin-p to 3.6.4
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on pull request #63:
URL: https://github.com/apache/maven-apache-parent/pull/63#issuecomment-1021211256
Ok, for me is ok add `dependencyManagement` without scope.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-apache-parent] kwin commented on pull request #63: [MPOM-288] - Update m-plugin-p to 3.6.4
Posted by GitBox <gi...@apache.org>.
kwin commented on pull request #63:
URL: https://github.com/apache/maven-apache-parent/pull/63#issuecomment-1023228117
Can anybody approve then? Currently the scope is not managed and it seems to be consensus to leave it like that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-apache-parent] slawekjaranowski commented on pull request #63: [MPOM-288] - Update m-plugin-p to 3.6.4
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on pull request #63:
URL: https://github.com/apache/maven-apache-parent/pull/63#issuecomment-1021211256
Ok, for me is ok add `dependencyManagement` without scope.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-apache-parent] ctubbsii commented on pull request #63: [MPOM-288] - Update m-plugin-p to 3.6.4
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #63:
URL: https://github.com/apache/maven-apache-parent/pull/63#issuecomment-1021154511
@slawekjaranowski wrote:
> Currently we have also defined dependency management for it in maven-parent with provided scope
>
> https://github.com/apache/maven-parent/blob/master/pom.xml#L965-L970
And, just as I said, people can't trust the convention breakage, so they override it anyway, making it redundant. Here's two examples:
* https://github.com/apache/maven-compiler-plugin/blob/master/pom.xml#L103
* https://github.com/apache/maven-clean-plugin/blob/master/pom.xml#L92
While I wouldn't recommend breaking conventions in `maven-plugins` parent POM either, it is at least limited to the Apache Maven plugin developers in that POM. If its scope were managed in this POM, it would break conventions for much wider set of projects across the ASF, and even projects outside the ASF.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-apache-parent] slachiewicz merged pull request #63: [MPOM-288] - Update m-plugin-p to 3.6.4
Posted by GitBox <gi...@apache.org>.
slachiewicz merged pull request #63:
URL: https://github.com/apache/maven-apache-parent/pull/63
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-apache-parent] kwin commented on a change in pull request #63: [MPOM-288] - Update m-plugin-p to 3.6.4
Posted by GitBox <gi...@apache.org>.
kwin commented on a change in pull request #63:
URL: https://github.com/apache/maven-apache-parent/pull/63#discussion_r788171242
##########
File path: pom.xml
##########
@@ -92,10 +92,21 @@ under the License.
<maven.compiler.source>${maven.compiler.target}</maven.compiler.source>
<maven.compiler.target>1.7</maven.compiler.target>
<surefire.version>2.22.2</surefire.version><!-- for surefire, failsafe and surefire-report -->
+ <maven.plugin.tools.version>3.6.4</maven.plugin.tools.version><!-- for m-plugin-p and maven-plugin-annotations -->
<assembly.tarLongFileMode>posix</assembly.tarLongFileMode>
<project.build.outputTimestamp>2021-07-14T15:10:38Z</project.build.outputTimestamp>
</properties>
+ <dependencyManagement>
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.maven.plugin-tools</groupId>
+ <artifactId>maven-plugin-annotations</artifactId>
+ <version>${maven.plugin.tools.version}</version>
+ </dependency>
Review comment:
This particular dependency should IMHO never be used with another scope, so this may warrant to set scope in depMgmt
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-apache-parent] slawekjaranowski commented on pull request #63: [MPOM-288] - Update m-plugin-p to 3.6.4
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on pull request #63:
URL: https://github.com/apache/maven-apache-parent/pull/63#issuecomment-1019569266
Currently we have also defined dependency management for it in maven-parent with provided scope
https://github.com/apache/maven-parent/blob/master/pom.xml#L965-L970
And another property is used for version defined:
`mavenPluginToolsVersion`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-apache-parent] kwin commented on a change in pull request #63: [MPOM-288] - Update m-plugin-p to 3.6.4
Posted by GitBox <gi...@apache.org>.
kwin commented on a change in pull request #63:
URL: https://github.com/apache/maven-apache-parent/pull/63#discussion_r787431660
##########
File path: pom.xml
##########
@@ -92,10 +92,21 @@ under the License.
<maven.compiler.source>${maven.compiler.target}</maven.compiler.source>
<maven.compiler.target>1.7</maven.compiler.target>
<surefire.version>2.22.2</surefire.version><!-- for surefire, failsafe and surefire-report -->
+ <maven.plugin.tools.version>3.6.4</maven.plugin.tools.version><!-- for m-plugin-p and maven-plugin-annotations -->
<assembly.tarLongFileMode>posix</assembly.tarLongFileMode>
<project.build.outputTimestamp>2021-07-14T15:10:38Z</project.build.outputTimestamp>
</properties>
+ <dependencyManagement>
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.maven.plugin-tools</groupId>
+ <artifactId>maven-plugin-annotations</artifactId>
+ <version>${maven.plugin.tools.version}</version>
+ </dependency>
Review comment:
Should the scope be managed as "provided" for this case?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-apache-parent] ctubbsii commented on a change in pull request #63: [MPOM-288] - Update m-plugin-p to 3.6.4
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #63:
URL: https://github.com/apache/maven-apache-parent/pull/63#discussion_r788361219
##########
File path: pom.xml
##########
@@ -92,10 +92,21 @@ under the License.
<maven.compiler.source>${maven.compiler.target}</maven.compiler.source>
<maven.compiler.target>1.7</maven.compiler.target>
<surefire.version>2.22.2</surefire.version><!-- for surefire, failsafe and surefire-report -->
+ <maven.plugin.tools.version>3.6.4</maven.plugin.tools.version><!-- for m-plugin-p and maven-plugin-annotations -->
<assembly.tarLongFileMode>posix</assembly.tarLongFileMode>
<project.build.outputTimestamp>2021-07-14T15:10:38Z</project.build.outputTimestamp>
</properties>
+ <dependencyManagement>
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.maven.plugin-tools</groupId>
+ <artifactId>maven-plugin-annotations</artifactId>
+ <version>${maven.plugin.tools.version}</version>
+ </dependency>
Review comment:
@kwin Even if it is extremely unlikely (I'm not going to say never, because people are creative), it's still going to create confusion, because people assume if the scope isn't specified, it is `compile`. Maven is built around a certain amount of convention. The more you break those conventions, the more it gets confusing.
For the users who might actually want it in the `compile` scope, you're really creating a problem for them, because now they have to explicitly add `<scope>compile</scope>`... *and* they probably have to leave a comment in for other developers, because lots of developers would just delete that line because they know it's not necessary most of the time.
If users actually want it to be `provided` (and yeah, they probably would much of the time for this one), then they're probably going to set it explicitly anyway. When they do, the one in the `<dependencyManagement>` section is redundant and adds no value.
So, best case scenario, when users are following conventions, it does not serve a purpose because it's redundant. Worst case scenario, you're really screwing with people's understanding of Maven because you've broken a pretty widely known convention.
The `<dependencyManagement>` section is useful when it helps with dependency convergence (version management, transitive dependency exclusions, etc.). But setting the scope there severely disrupts conventions and has substantial downstream impact on anybody using this as a parent POM, because it quite literally alters their class paths.
I strongly advise against breaking conventions like this in such a widely used parent POM that could break people's understanding of how Maven works, or messes with which jars are available in which class paths. In general, I strongly recommend against setting the scope in any `<dependencyManagement>` section, unless it's `<type>pom</type>` and `<scope>import</scope>`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-apache-parent] Tibor17 commented on pull request #63: [MPOM-288] - Update m-plugin-p to 3.6.4
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #63:
URL: https://github.com/apache/maven-apache-parent/pull/63#issuecomment-1021215526
The scope should not be in MPOM.
The MPOM has certain purpose to help and not to break via restrictions.
Simply dependency management declares versions, the same with plugin management.
Plugins section is different story.
The way that we declare versions of dependencies has certain purpose - we prove that the JDK version and Maven API/versions and plugins development would be in consistent state. It's the developer's responsibility to say what dependency would be on certain type of classpath (compilation, test, runtime). The more restrictions of scope we provide in MPOM the more investigation time would be spent on finding out this root cause in MPOM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org