michael-o commented on code in PR #104:
URL: https://github.com/apache/maven-release/pull/104#discussion_r1133229249
##########
maven-release-api/src/main/java/org/apache/maven/shared/release/versions/VersionParseException.java:
##########
@@ -34,4 +34,14 @@ public VersionParseException( String message )
{
super( message );
}
+
+ /**
+ * <p>Constructor for VersionParseException.</p>
+ *
+ * @param message a {@link java.lang.String} object
+ */
+ public VersionParseException( String message, Exception e )
Review Comment:
`Throwable` like the superclass?
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/config/ReleaseDescriptorBuilder.java:
##########
@@ -401,6 +401,18 @@ public ReleaseDescriptorBuilder setProjectVersionPolicyId(
String projectVersion
return this;
}
+ /**
+ * <p>setProjectVersionPolicyConfig.</p>
+ *
+ * @param setProjectVersionPolicyConfig a {@link java.lang.String} object
+ * @return a {@link
org.apache.maven.shared.release.config.ReleaseDescriptorBuilder} object
+ */
+ public ReleaseDescriptorBuilder setProjectVersionPolicyConfig( String
setProjectVersionPolicyConfig )
+ {
+ releaseDescriptor.setProjectVersionPolicyConfig(
setProjectVersionPolicyConfig );
Review Comment:
So if this would be the `XmlPlexusConfiguration` you'd be force to parse the
serialized config into that type?
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/config/ReleaseUtils.java:
##########
@@ -156,6 +156,10 @@ public static void copyPropertiesToReleaseDescriptor(
Properties properties, Rel
{
builder.setProjectVersionPolicyId( properties.getProperty(
"projectVersionPolicyId" ) );
}
+ if ( properties.containsKey( "projectVersionPolicyConfig" ) )
+ {
+ builder.setProjectVersionPolicyConfig( properties.getProperty(
"projectVersionPolicyConfig" ) );
Review Comment:
Here as well, I guess?!
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/phase/AbstractMapVersionsPhase.java:
##########
@@ -347,16 +362,42 @@ private String getContextString( ReleaseDescriptor
releaseDescriptor )
return "new development";
}
- private String resolveSuggestedVersion( String baseVersion, String
policyId )
+ private String resolveSuggestedVersion( String baseVersion,
ReleaseDescriptor releaseDescriptor )
throws PolicyException, VersionParseException
{
+ String policyId = releaseDescriptor.getProjectVersionPolicyId();
VersionPolicy policy = versionPolicies.get( policyId );
if ( policy == null )
{
throw new PolicyException( "Policy '" + policyId + "' is unknown,
available: " + versionPolicies.keySet() );
}
VersionPolicyRequest request = new VersionPolicyRequest().setVersion(
baseVersion );
+
+ if ( releaseDescriptor.getProjectVersionPolicyConfig() != null )
+ {
+ request.setConfig(
releaseDescriptor.getProjectVersionPolicyConfig().toString() );
+ }
+ request.setWorkingDirectory( releaseDescriptor.getWorkingDirectory() );
+
+ if ( scmRepositoryConfigurator != null &&
releaseDescriptor.getScmSourceUrl() != null )
+ {
+ try
+ {
+ ScmRepository repository = scmRepositoryConfigurator
+ .getConfiguredRepository( releaseDescriptor, new
Settings() );
+
+ ScmProvider provider = scmRepositoryConfigurator
+ .getRepositoryProvider( repository );
+
+ request.setScmRepository( repository );
+ request.setScmProvider( provider );
+ }
+ catch ( ScmRepositoryException | NoSuchScmProviderException e )
+ {
+ getLogger().warn( "Next Version will NOT be based on the
version control: {}", e.getMessage() );
Review Comment:
This loses stack trace, we try to apply the following scheme:
```
if (log.isWarn())
getLogger().warn( "Next Version will NOT be based on the version control")
else if (log.isDebug())
getLogger().warn( "Next Version will NOT be based on the version control", e)
```
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/phase/AbstractMapVersionsPhase.java:
##########
@@ -67,9 +75,16 @@
* @author <a href="mailto:br...@apache.org";>Brett Porter</a>
* @author Robert Scholte
*/
+@Component( role = ReleasePhase.class, hint = "map-release-versions" )
public abstract class AbstractMapVersionsPhase
Review Comment:
This is then inherited to the non-abstract class?
##########
maven-release-api/src/main/java/org/apache/maven/shared/release/config/ReleaseDescriptor.java:
##########
@@ -426,6 +426,13 @@
*/
String getProjectVersionPolicyId();
+ /**
+ * Get the (optional) config for the VersionPolicy implementation used to
calculate the project versions.
+ *
+ * @return The parsed XML of the provided config (an instance of
XmlPlexusConfiguration) or null.
+ */
+ Object getProjectVersionPolicyConfig();
+
Review Comment:
I wonder if this is always going to be `XmlPlexusConfiguration` why not make
it `XmlPlexusConfiguration`? Too much overhead? I mean the Javadoc is clear on
this, no?
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/phase/AbstractMapVersionsPhase.java:
##########
@@ -347,16 +362,42 @@ private String getContextString( ReleaseDescriptor
releaseDescriptor )
return "new development";
}
- private String resolveSuggestedVersion( String baseVersion, String
policyId )
+ private String resolveSuggestedVersion( String baseVersion,
ReleaseDescriptor releaseDescriptor )
throws PolicyException, VersionParseException
{
+ String policyId = releaseDescriptor.getProjectVersionPolicyId();
VersionPolicy policy = versionPolicies.get( policyId );
if ( policy == null )
{
throw new PolicyException( "Policy '" + policyId + "' is unknown,
available: " + versionPolicies.keySet() );
}
VersionPolicyRequest request = new VersionPolicyRequest().setVersion(
baseVersion );
+
+ if ( releaseDescriptor.getProjectVersionPolicyConfig() != null )
+ {
+ request.setConfig(
releaseDescriptor.getProjectVersionPolicyConfig().toString() );
+ }
+ request.setWorkingDirectory( releaseDescriptor.getWorkingDirectory() );
+
+ if ( scmRepositoryConfigurator != null &&
releaseDescriptor.getScmSourceUrl() != null )
+ {
+ try
+ {
+ ScmRepository repository = scmRepositoryConfigurator
+ .getConfiguredRepository( releaseDescriptor, new
Settings() );
Review Comment:
Settings don't matter if they are requied?
--
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-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
