[chore] Release 9.11.0 Take 2 - #1246
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces configurations to skip integration tests (skipITs) during releases and deployments. The review feedback points out several opportunities to clean up the changes by removing redundant configurations, such as the explicit skipITs flag in the publish script and the redundant configuration block in the maven-failsafe-plugin. Additionally, it is recommended to define a default value for skipTests in the POM properties to avoid unresolved property placeholders.
| mvn -B clean deploy \ | ||
| -Dcheckstyle.skip \ | ||
| -DskipTests \ | ||
| -DskipITs \ |
There was a problem hiding this comment.
The addition of -DskipITs is redundant here. Since -DskipTests is already passed, and pom.xml defines <skipITs>${skipTests}</skipITs>, skipITs will automatically be set to true. Furthermore, the release profile (activated via -Prelease) also explicitly sets <skipITs>true</skipITs>. You can safely remove this line to keep the script clean.
| <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
| <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding> | ||
| <skipUTs>${skipTests}</skipUTs> | ||
| <skipITs>${skipTests}</skipITs> |
There was a problem hiding this comment.
To avoid passing unresolved property placeholders (like ${skipTests}) when the skipTests property is not explicitly defined on the command line, it is recommended to define a default value for <skipTests> in the <properties> block.
| <skipITs>${skipTests}</skipITs> | |
| <skipTests>false</skipTests> | |
| <skipITs>${skipTests}</skipITs> |
| <configuration> | ||
| <skipITs>${skipITs}</skipITs> | ||
| </configuration> |
There was a problem hiding this comment.
The <configuration> block explicitly setting <skipITs>${skipITs}</skipITs> is redundant because the maven-failsafe-plugin automatically binds its skipITs parameter to the skipITs system/Maven property by default. You can safely remove this configuration block to keep the POM file cleaner.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the pom.xml file to introduce the skipTests and skipITs properties, defaulting skipTests to false and setting skipITs to inherit from skipTests. Additionally, it configures the release profile to skip integration tests by setting skipITs to true. There are no review comments to address, and I have no additional feedback to provide.
2 participants
[chore] Release 9.11.0 take 2
In #1244, maven-failsafe-plugin was upgraded to 3.6.0. In Failsafe 3.6.0, the parameter for
skipTestswas unbound from the CLI propertyskipTests. Consequently, passing only-DskipTestsskipped unit tests (Surefire) but no longer skipped integration tests (Failsafe). Failsafe attempted to run all integration tests during deploy, failing becauseintegration_cert.jsonis not present in the environment.