Skip to content
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

MRELEASE-979 - use split policies #15

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -123,6 +123,8 @@ public static ReleaseDescriptor merge( ReleaseDescriptor mergeInto, ReleaseDescr

mergeInto.setProjectVersionPolicyId(
mergeDefault( mergeInto.getProjectVersionPolicyId(), toBeMerged.getProjectVersionPolicyId() ) );
mergeInto.setProjectNamingPolicyId(
mergeDefault( mergeInto.getProjectNamingPolicyId(), toBeMerged.getProjectNamingPolicyId() ) );

return mergeInto;
}
Expand Down Expand Up @@ -163,6 +165,7 @@ public static ReleaseDescriptor copyPropertiesToReleaseDescriptor( Properties pr
releaseDescriptor.setPreparationGoals( properties.getProperty( "preparationGoals" ) );
releaseDescriptor.setCompletionGoals( properties.getProperty( "completionGoals" ) );
releaseDescriptor.setProjectVersionPolicyId( properties.getProperty( "projectVersionPolicyId" ) );
releaseDescriptor.setProjectNamingPolicyId( properties.getProperty( "projectNamingPolicyId" ) );
String snapshotReleasePluginAllowedStr = properties.getProperty( "exec.snapshotReleasePluginAllowed" );
releaseDescriptor.setSnapshotReleasePluginAllowed( snapshotReleasePluginAllowedStr == null
? false
Expand Down
Expand Up @@ -74,7 +74,7 @@ public class InputVariablesPhase
*/
@Requirement
private ScmRepositoryConfigurator scmRepositoryConfigurator;

/**
* Component used for custom or default naming policy
*/
Expand Down Expand Up @@ -134,9 +134,11 @@ public ReleaseResult execute( ReleaseDescriptor releaseDescriptor, ReleaseEnviro
throw new ReleaseExecutionException( "Project tag cannot be selected if version is not yet mapped" );
}

String defaultTag;
String defaultTag = null;
String scmTagNameFormat = releaseDescriptor.getScmTagNameFormat();
if ( scmTagNameFormat != null )

// Only apply the scmTagName format for tag operations, not branching.
if ( !branchOperation && scmTagNameFormat != null )
{
Interpolator interpolator = new StringSearchInterpolator( "@{", "}" );
List<String> possiblePrefixes = java.util.Arrays.asList( "project", "pom" );
Expand All @@ -156,7 +158,8 @@ public ReleaseResult execute( ReleaseDescriptor releaseDescriptor, ReleaseEnviro
"Could not interpolate specified tag name format: " + scmTagNameFormat, e );
}
}
else

if ( defaultTag == null )
{
try
{
Expand All @@ -166,7 +169,7 @@ public ReleaseResult execute( ReleaseDescriptor releaseDescriptor, ReleaseEnviro
catch ( PolicyException e )
{
throw new ReleaseExecutionException( e.getMessage(), e );
}
}
}

ScmProvider provider = null;
Expand Down Expand Up @@ -207,9 +210,16 @@ public ReleaseResult execute( ReleaseDescriptor releaseDescriptor, ReleaseEnviro
e );
}
}
else if ( branchOperation )
else if ( defaultTag == null )
{
throw new ReleaseExecutionException( "No branch name was given." );
if ( branchOperation )
{
throw new ReleaseExecutionException( "No branch name was given." );
}
else
{
throw new ReleaseExecutionException( "No tag name was given." );
}
}
else
{
Expand All @@ -236,7 +246,7 @@ public ReleaseResult simulate( ReleaseDescriptor releaseDescriptor, ReleaseEnvir

return result;
}

private String resolveSuggestedName( String policyId, String version, MavenProject project )
throws PolicyException
{
Expand Down
@@ -0,0 +1,41 @@
package org.apache.maven.shared.release.policies;

/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import org.apache.maven.shared.release.policy.PolicyException;
import org.apache.maven.shared.release.policy.naming.NamingPolicy;
import org.apache.maven.shared.release.policy.naming.NamingPolicyRequest;
import org.apache.maven.shared.release.policy.naming.NamingPolicyResult;
import org.codehaus.plexus.component.annotations.Component;

/**
*
* @since 3.0.0
*/
@Component( role = NamingPolicy.class, hint = "default-branch" )
public class DefaultBranchNamingPolicy implements NamingPolicy
{
@Override
public NamingPolicyResult getName( NamingPolicyRequest request )
throws PolicyException
{
return new NamingPolicyResult().setName( null );
}
}
Expand Up @@ -68,7 +68,7 @@ public void testInputVariablesInteractive()

List<MavenProject> reactorProjects = Collections.singletonList( createProject( "artifactId", "1.0" ) );

ReleaseDescriptor releaseDescriptor = new ReleaseDescriptor();
ReleaseDescriptor releaseDescriptor = createReleaseDescriptor();
releaseDescriptor.mapReleaseVersion( "groupId:artifactId", "1.0" );
releaseDescriptor.setScmSourceUrl( "scm:svn:file://localhost/tmp/scm-repo" );

Expand All @@ -79,7 +79,7 @@ public void testInputVariablesInteractive()
assertEquals( "Check tag", "tag-value", releaseDescriptor.getScmReleaseLabel() );

// prepare
releaseDescriptor = new ReleaseDescriptor();
releaseDescriptor = createReleaseDescriptor();
releaseDescriptor.mapReleaseVersion( "groupId:artifactId", "1.0" );
releaseDescriptor.setScmSourceUrl( "scm:svn:file://localhost/tmp/scm-repo" );

Expand All @@ -99,7 +99,7 @@ public void testUnmappedVersion()
{
List<MavenProject> reactorProjects = Collections.singletonList( createProject( "artifactId", "1.0" ) );

ReleaseDescriptor releaseDescriptor = new ReleaseDescriptor();
ReleaseDescriptor releaseDescriptor = createReleaseDescriptor();

try
{
Expand All @@ -112,7 +112,7 @@ public void testUnmappedVersion()
assertNull( "check no cause", e.getCause() );
}

releaseDescriptor = new ReleaseDescriptor();
releaseDescriptor = createReleaseDescriptor();

try
{
Expand All @@ -136,7 +136,7 @@ public void testInputVariablesNonInteractiveConfigured()

List<MavenProject> reactorProjects = Collections.singletonList( createProject( "artifactId", "1.0" ) );

ReleaseDescriptor releaseDescriptor = new ReleaseDescriptor();
ReleaseDescriptor releaseDescriptor = createReleaseDescriptor();
releaseDescriptor.setInteractive( false );
releaseDescriptor.setScmReleaseLabel( "tag-value" );

Expand All @@ -147,7 +147,7 @@ public void testInputVariablesNonInteractiveConfigured()
assertEquals( "Check tag", "tag-value", releaseDescriptor.getScmReleaseLabel() );

// prepare
releaseDescriptor = new ReleaseDescriptor();
releaseDescriptor = createReleaseDescriptor();
releaseDescriptor.setInteractive( false );
releaseDescriptor.setScmReleaseLabel( "simulated-tag-value" );

Expand All @@ -171,7 +171,7 @@ public void testInputVariablesInteractiveConfigured()

List<MavenProject> reactorProjects = Collections.singletonList( createProject( "artifactId", "1.0" ) );

ReleaseDescriptor releaseDescriptor = new ReleaseDescriptor();
ReleaseDescriptor releaseDescriptor = createReleaseDescriptor();
releaseDescriptor.setScmReleaseLabel( "tag-value" );

// execute
Expand All @@ -181,7 +181,7 @@ public void testInputVariablesInteractiveConfigured()
assertEquals( "Check tag", "tag-value", releaseDescriptor.getScmReleaseLabel() );

// prepare
releaseDescriptor = new ReleaseDescriptor();
releaseDescriptor = createReleaseDescriptor();
releaseDescriptor.setScmReleaseLabel( "simulated-tag-value" );

// execute
Expand All @@ -206,7 +206,7 @@ public void testPrompterException()

List<MavenProject> reactorProjects = Collections.singletonList( createProject( "artifactId", "1.0" ) );

ReleaseDescriptor releaseDescriptor = new ReleaseDescriptor();
ReleaseDescriptor releaseDescriptor = createReleaseDescriptor();
releaseDescriptor.mapReleaseVersion( "groupId:artifactId", "1.0" );
releaseDescriptor.setScmSourceUrl( "scm:svn:file://localhost/tmp/scm-repo" );

Expand All @@ -223,7 +223,7 @@ public void testPrompterException()
}

// prepare
releaseDescriptor = new ReleaseDescriptor();
releaseDescriptor = createReleaseDescriptor();
releaseDescriptor.mapReleaseVersion( "groupId:artifactId", "1.0" );
releaseDescriptor.setScmSourceUrl( "scm:svn:file://localhost/tmp/scm-repo" );

Expand Down Expand Up @@ -261,7 +261,7 @@ public void testEmptyBranchName()

List<MavenProject> reactorProjects = Collections.singletonList( createProject( "artifactId", "1.0" ) );

ReleaseDescriptor releaseDescriptor = new ReleaseDescriptor();
ReleaseDescriptor releaseDescriptor = createReleaseDescriptor();
releaseDescriptor.setInteractive( false );
releaseDescriptor.setScmReleaseLabel( null );
releaseDescriptor.mapReleaseVersion( "groupId:artifactId", "1.0" );
Expand All @@ -280,7 +280,7 @@ public void testEmptyBranchName()
}

// prepare
releaseDescriptor = new ReleaseDescriptor();
releaseDescriptor = createReleaseDescriptor();
releaseDescriptor.setInteractive( false );
releaseDescriptor.setScmReleaseLabel( null );
releaseDescriptor.mapReleaseVersion( "groupId:artifactId", "1.0" );
Expand Down Expand Up @@ -310,4 +310,11 @@ private static MavenProject createProject( String artifactId, String version )
model.setVersion( version );
return new MavenProject( model );
}

private static ReleaseDescriptor createReleaseDescriptor()
{
ReleaseDescriptor releaseDescriptor = new ReleaseDescriptor();
releaseDescriptor.setProjectNamingPolicyId("default-branch");
return releaseDescriptor;
}
}
Expand Up @@ -199,12 +199,12 @@ public class BranchReleaseMojo
private String projectVersionPolicyId;

/**
* The role-hint for the NamingPolicy implementation used to calculate the project branch and tag names.
* The role-hint for the NamingPolicy implementation used to calculate the project names.
*
* @since 3.0.0
*/
@Parameter( defaultValue = "default", property = "projectNamingPolicyId" )
private String projectNamingPolicyId;
@Parameter( defaultValue = "default-branch", property = "projectBranchNamingPolicyId" )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defaultValue should stay "default", property should stay "projectNamingPolicyId", since you will never run release:prepare and release:branch. Only the name of the field has to be different, so you can configure both inside your pom.

Copy link
Author

@hgschmie hgschmie Mar 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the default value is "default", then it selects the same policy as the default tag naming policy because there is only a single map with policies. That is the whole point of this change, as the default policy is different for tags (there it uses <artifact>-<version> if no tag was given) and branches (there it provides no default therefore a branch request fails if no name is given). As you insisted to moving this naming strategy into the DefaultNamingStrategy and refused any approach to separate those out, what is your suggestion to continue?

I am at a loss here. I am going back and forth through these changes and every time, you come back saying "this needs to be different, not this way". There are two default policies and you block any approach to acknowledge this fact.

private String projectBranchNamingPolicyId;

/**
* {@inheritDoc}
Expand All @@ -230,7 +230,7 @@ public void execute()
config.setDefaultDevelopmentVersion( developmentVersion );
config.setSuppressCommitBeforeTagOrBranch( suppressCommitBeforeBranch );
config.setProjectVersionPolicyId( projectVersionPolicyId );
config.setProjectNamingPolicyId( projectNamingPolicyId );
config.setProjectNamingPolicyId( projectBranchNamingPolicyId );

// Create a config containing values from the session properties (ie command line properties with cli).
ReleaseDescriptor sysPropertiesConfig
Expand Down
Expand Up @@ -226,12 +226,12 @@ public class PrepareReleaseMojo
private String projectVersionPolicyId;

/**
* The role-hint for the NamingPolicy implementation used to calculate the project branch and tag names.
* The role-hint for the NamingPolicy implementation used to calculate the project tag names.
*
* @since 3.0.0
*/
@Parameter( defaultValue = "default", property = "projectNamingPolicyId" )
private String projectNamingPolicyId;
@Parameter( defaultValue = "default", property = "projectTagNamingPolicyId" )
private String projectTagNamingPolicyId;

/**
* {@inheritDoc}
Expand Down Expand Up @@ -273,7 +273,7 @@ protected void prepareRelease( boolean generateReleasePoms )
config.setSuppressCommitBeforeTagOrBranch( suppressCommitBeforeTag );
config.setWaitBeforeTagging( waitBeforeTagging );
config.setProjectVersionPolicyId( projectVersionPolicyId );
config.setProjectNamingPolicyId( projectNamingPolicyId );
config.setProjectNamingPolicyId( projectTagNamingPolicyId );

if ( checkModificationExcludeList != null )
{
Expand Down