[MRELEASE-839]: Unable to supply tag to release for release:perform#149
[MRELEASE-839]: Unable to supply tag to release for release:perform#149edwardUL99 wants to merge 1 commit intoapache:masterfrom
Conversation
| @Mojo( name = "perform", aggregator = true, requiresProject = false ) | ||
| public class PerformReleaseMojo | ||
| extends AbstractReleaseMojo | ||
| extends AbstractScmReleaseMojo |
There was a problem hiding this comment.
see #145 (comment). This exposes a lot more unused parameters, not just tag
There was a problem hiding this comment.
@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?
|
Resolve #1040 |
There was a problem hiding this comment.
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> |
There was a problem hiding this comment.
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() ); |
There was a problem hiding this comment.
Duplicate assertion. Line 82 already contains assertNotNull( argument.getValue().getReleaseDescriptorBuilder() );. This duplicate assertion should be removed.
| assertNotNull( argument.getValue().getReleaseDescriptorBuilder() ); |
|
|
||
| return scm; | ||
| } | ||
|
|
There was a problem hiding this comment.
This method overrides MavenProjectStub.getOriginalModel; it is advisable to add an Override annotation.
| @Override |
| @@ -49,6 +50,26 @@ public Model getModel() | |||
| return model; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
This method overrides MavenProjectStub.getScm; it is advisable to add an Override annotation.
| @Override |
3 participants
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