You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@datasketches.apache.org by "tisonkun (via GitHub)" <gi...@apache.org> on 2026/01/07 06:11:21 UTC
[PR] Allow group override (datasketches-java)
tisonkun opened a new pull request, #711:
URL: https://github.com/apache/datasketches-java/pull/711
(no comment)
--
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: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [PR] Allow override surefire plugin group (datasketches-java)
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #711:
URL: https://github.com/apache/datasketches-java/pull/711#discussion_r2670368334
##########
pom.xml:
##########
@@ -129,6 +129,20 @@ under the License.
</dependencies>
<build>
+ <pluginManagement>
+ <plugins>
+ <plugin>
+ <!-- Apache Parent pom, pluginManagement-->
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <!-- by default, exclude all snapshot targets -->
+ <!-- this will be overridden on dedicated profile -->
+ <excludedGroups>${testng.generate-java-files},${testng.check-cpp-files},${testng.check-go-files},${testng.check-cpp-historical-files}</excludedGroups>
+ </configuration>
+ </plugin>
+ </plugins>
+ </pluginManagement>
Review Comment:
Note that we use `/pluginManagement/plugins/plugin` to override the exclude group of surefire plugin:
```xml
<profile>
<id>generate-java-files</id>
<build>
<pluginManagement>
<plugins>
<plugin>
<!-- Apache Parent pom, pluginManagement-->
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<groups>${testng.generate-java-files}</groups>
<excludedGroups>${testng.check-cpp-files},${testng.check-go-files},${testng.check-cpp-historical-files}</excludedGroups>
</configuration>
</plugin>
</plugins>
</pluginManagement>
</build>
</profile>
```
So this PR changes all the configurations to `/pluginManagement` for proper overriding.
> configuration will be overridden by the surefire plugin specified under <build><plugins>.
Yes. The current `pom.xml` before this patch, would always use:
```xml
<excludedGroups>${testng.generate-java-files},${testng.check-cpp-files},${testng.check-go-files},
${testng.check-cpp-historical-files}</excludedGroups> <!-- do not indent -->
```
Because it is the final config. Thus, any profile won't be applied actually.
--
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: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [PR] Allow override surefire plugin group (datasketches-java)
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho commented on code in PR #711:
URL: https://github.com/apache/datasketches-java/pull/711#discussion_r2669590171
##########
pom.xml:
##########
@@ -274,8 +288,6 @@ under the License.
<useManifestOnlyJar>false</useManifestOnlyJar>
<redirectTestOutputToFile>true</redirectTestOutputToFile>
<reportsDirectory>${project.build.directory}/test-output/${maven.build.timestamp}</reportsDirectory>
- <excludedGroups>${testng.generate-java-files},${testng.check-cpp-files},${testng.check-go-files},
-${testng.check-cpp-historical-files}</excludedGroups> <!-- do not indent -->
Review Comment:
I had set this up so that an ordinary "mvn test" would not generate the java files and run the tests against the other languages. These are run with specific profiles.
Now that we are adding more languages into the mix, I am not clear on what you have in mind.
--
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: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [PR] Allow override surefire plugin group (datasketches-java)
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho commented on code in PR #711:
URL: https://github.com/apache/datasketches-java/pull/711#discussion_r2669609030
##########
pom.xml:
##########
@@ -129,6 +129,20 @@ under the License.
</dependencies>
<build>
+ <pluginManagement>
+ <plugins>
+ <plugin>
+ <!-- Apache Parent pom, pluginManagement-->
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <!-- by default, exclude all snapshot targets -->
+ <!-- this will be overridden on dedicated profile -->
+ <excludedGroups>${testng.generate-java-files},${testng.check-cpp-files},${testng.check-go-files},${testng.check-cpp-historical-files}</excludedGroups>
+ </configuration>
+ </plugin>
+ </plugins>
+ </pluginManagement>
Review Comment:
The ds-java POM does not have any maven modules so there is no inheritance. I also was warned that a configuration specification in a plugin under pluginManagement may be ignored by some plugins. Also this configuration will be overridden by the surefire plugin specified under \<build\>\<plugins\>. So what is achieved by putting this plugin under pluginManagement with a different configuration?
--
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: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [PR] Allow override surefire plugin group (datasketches-java)
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho merged PR #711:
URL: https://github.com/apache/datasketches-java/pull/711
--
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: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [PR] Allow override surefire plugin group (datasketches-java)
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho commented on code in PR #711:
URL: https://github.com/apache/datasketches-java/pull/711#discussion_r2669609030
##########
pom.xml:
##########
@@ -129,6 +129,20 @@ under the License.
</dependencies>
<build>
+ <pluginManagement>
+ <plugins>
+ <plugin>
+ <!-- Apache Parent pom, pluginManagement-->
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <!-- by default, exclude all snapshot targets -->
+ <!-- this will be overridden on dedicated profile -->
+ <excludedGroups>${testng.generate-java-files},${testng.check-cpp-files},${testng.check-go-files},${testng.check-cpp-historical-files}</excludedGroups>
+ </configuration>
+ </plugin>
+ </plugins>
+ </pluginManagement>
Review Comment:
The ds-java POM does not have any maven modules so there is no inheritance. I also was warned that a configuration specification in a plugin under pluginManagement may be ignored by some plugins. So what is achieved by putting this plugin under pluginManagement?
I also don't see this plugin listed under the <build><plugins>.
--
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: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [PR] Allow override surefire plugin group (datasketches-java)
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho commented on code in PR #711:
URL: https://github.com/apache/datasketches-java/pull/711#discussion_r2669609030
##########
pom.xml:
##########
@@ -129,6 +129,20 @@ under the License.
</dependencies>
<build>
+ <pluginManagement>
+ <plugins>
+ <plugin>
+ <!-- Apache Parent pom, pluginManagement-->
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <!-- by default, exclude all snapshot targets -->
+ <!-- this will be overridden on dedicated profile -->
+ <excludedGroups>${testng.generate-java-files},${testng.check-cpp-files},${testng.check-go-files},${testng.check-cpp-historical-files}</excludedGroups>
+ </configuration>
+ </plugin>
+ </plugins>
+ </pluginManagement>
Review Comment:
The ds-java POM does not have any maven modules so there is no inheritance. I also was warned that a configuration specification in a plugin under pluginManagement may be ignored by some plugins. Also this configuration will be overridden by the surefire plugin specified under <build><plugins>. So what is achieved by putting this plugin under pluginManagement with a different configuration?
--
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: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [PR] Allow override surefire plugin group (datasketches-java)
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #711:
URL: https://github.com/apache/datasketches-java/pull/711#discussion_r2670371586
##########
pom.xml:
##########
@@ -274,8 +288,6 @@ under the License.
<useManifestOnlyJar>false</useManifestOnlyJar>
<redirectTestOutputToFile>true</redirectTestOutputToFile>
<reportsDirectory>${project.build.directory}/test-output/${maven.build.timestamp}</reportsDirectory>
- <excludedGroups>${testng.generate-java-files},${testng.check-cpp-files},${testng.check-go-files},
-${testng.check-cpp-historical-files}</excludedGroups> <!-- do not indent -->
Review Comment:
See https://github.com/apache/datasketches-java/pull/711#discussion_r2670368334.
This config would be still applied, but from the `/pluginManagement/plugins/plugin` section that can be override.
This PR is a no change for developer who only runs `mvn test`.
--
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: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [PR] Allow override surefire plugin group (datasketches-java)
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #711:
URL: https://github.com/apache/datasketches-java/pull/711#discussion_r2670369663
##########
pom.xml:
##########
@@ -129,6 +129,20 @@ under the License.
</dependencies>
<build>
+ <pluginManagement>
+ <plugins>
+ <plugin>
+ <!-- Apache Parent pom, pluginManagement-->
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <!-- by default, exclude all snapshot targets -->
+ <!-- this will be overridden on dedicated profile -->
+ <excludedGroups>${testng.generate-java-files},${testng.check-cpp-files},${testng.check-go-files},${testng.check-cpp-historical-files}</excludedGroups>
+ </configuration>
+ </plugin>
+ </plugins>
+ </pluginManagement>
Review Comment:
An alternative is to always use `/plugins/plugin` for configuring the surefire plugin in every profile.
--
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: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org