-
Notifications
You must be signed in to change notification settings - Fork 143
[MRELEASE-839]: Unable to supply tag to release for release:perform #149
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
base: master
Are you sure you want to change the base?
Conversation
| @Mojo( name = "perform", aggregator = true, requiresProject = false ) | ||
| public class PerformReleaseMojo | ||
| extends AbstractReleaseMojo | ||
| extends AbstractScmReleaseMojo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see #145 (comment). This exposes a lot more unused parameters, not just tag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kwin I see your point. What's the best course of action? Wait for 145 to be resolved, implement the changes suggested by 145, or just add tag property to perform release mojo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@michael-o Please advise how to continue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
Resolve #1040 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for configuring a custom tag in the release:perform goal by making PerformReleaseMojo extend AbstractScmReleaseMojo instead of AbstractReleaseMojo, which provides access to SCM-related parameters including the tag parameter.
- Changed
PerformReleaseMojoto extendAbstractScmReleaseMojoto enable tag parameter configuration - Updated all test resources and test cases to verify tag configuration is properly propagated
- Refactored test stubs to consolidate SCM configuration in the base
MavenProjectStubclass
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| maven-release-plugin/src/main/java/org/apache/maven/plugins/release/PerformReleaseMojo.java | Changed parent class to AbstractScmReleaseMojo to enable tag parameter support |
| maven-release-plugin/src/test/resources/mojos/perform/*.xml | Added <tag>test-tag</tag> configuration to all test resource files |
| maven-release-plugin/src/test/java/org/apache/maven/plugins/release/PerformReleaseMojoTest.java | Added assertTag() helper method and verification calls in all test methods |
| maven-release-plugin/src/test/java/org/apache/maven/plugins/release/stubs/MavenProjectStub.java | Added getScm() and getOriginalModel() methods to provide SCM configuration |
| maven-release-plugin/src/test/java/org/apache/maven/plugins/release/stubs/FlatMultiModuleMavenProjectStub.java | Refactored to extend project's MavenProjectStub instead of Maven testing stub, removing duplicated SCM methods |
| maven-release-plugin/src/it/projects/perform/MRELEASE-839/* | Added new integration test to verify tag configuration works in the perform goal |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <modelVersion>4.0.0</modelVersion> | ||
|
|
||
| <groupId>org.apache.maven.plugin.release.its</groupId> | ||
| <artifactId>mrelease-832</artifactId> |
Copilot
AI
Nov 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The artifact ID should be mrelease-839 to match the issue number and directory name (MRELEASE-839), but it's currently set to mrelease-832 which appears to be a copy-paste error from the MRELEASE-832 test.
| <artifactId>mrelease-832</artifactId> | |
| <artifactId>mrelease-839</artifactId> |
| assertNotNull( argument.getValue().getReleaseEnvironment() ); | ||
| assertNotNull( argument.getValue().getReactorProjects() ); | ||
| assertEquals( Boolean.FALSE, argument.getValue().getDryRun() ); | ||
| assertNotNull( argument.getValue().getReleaseDescriptorBuilder() ); |
Copilot
AI
Nov 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate assertion. Line 82 already contains assertNotNull( argument.getValue().getReleaseDescriptorBuilder() );. This duplicate assertion should be removed.
| assertNotNull( argument.getValue().getReleaseDescriptorBuilder() ); |
|
|
||
| return scm; | ||
| } | ||
|
|
Copilot
AI
Nov 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method overrides MavenProjectStub.getOriginalModel; it is advisable to add an Override annotation.
| @Override |
| } | ||
| return model; | ||
| } | ||
|
|
Copilot
AI
Nov 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method overrides MavenProjectStub.getScm; it is advisable to add an Override annotation.
| @Override |
Following this checklist to help us incorporate your
contribution quickly and easily:
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
[MJAVADOC-XXX] - Fixes bug in ApproximateQuantiles,where you replace
MJAVADOC-XXXwith the appropriate JIRA issue. Best practiceis to use the JIRA issue title in the pull request title and in the first line of the
commit message.
mvn clean verify -Prun-itsto make sure basic checks pass. A more thorough check willbe performed on your pull request automatically.
If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.
I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004
In any other case, please file an Apache Individual Contributor License Agreement.
This PR introduces a fix which allows a
tagto be supplied either using a-Dtag=<tag>flag or passing the tag into the<configuration><tag>tag</tag></configuration>section of the release plugin in the pom. ThePerformReleasePojopreviously incorrectly extendedAbstractReleaseMojowhere it now extendsAbstractScmReleaseMojo. This required some small changes to the unit tests where I updated the stub files to correct a NPE error and to also assert that the ScmReleaseLabel was being populated correctly. I also added a directoryMRELEASE-839under it/projects/perform which contains the integration test for this issue.From the Jira, the command to get the fixed changes is:
Replacing the connectionUrl with correct SCM and required tag. I have tested that it works with both localCheckout set to true and false