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

Allow the use of snapshot versions in release and hotfix while are opened #105

Conversation

alsanrum
Copy link
Contributor

Issue #81 and issue #98

* @since 1.9.1
*/
@Parameter(property = "useSnapshotInHotfix", defaultValue = "false")
protected boolean useSnapshotInHotfix;
Copy link
Owner

Choose a reason for hiding this comment

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

It is better to put parameters in the appropriate goals.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put them in the abstract class to avoid duplication.
I have seen that you have the properties in each goal.
I change it.

@@ -223,9 +223,23 @@ public void execute() throws MojoExecutionException, MojoFailureException {
// get current project version from pom
final String currentVersion = getCurrentProjectVersion();

if ((commitDevelopmentVersionAtStart && useSnapshotInRelease) && ArtifactUtils.isSnapshot(currentVersion)) {
Copy link
Owner

Choose a reason for hiding this comment

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

Why commitDevelopmentVersionAtStart should be enabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I remove this check.

@@ -179,10 +180,14 @@ public void execute() throws MojoExecutionException, MojoFailureException {
}

if (commitDevelopmentVersionAtStart) {
String projectVersion = releaseVersion;
if (useSnapshotInRelease && !ArtifactUtils.isSnapshot(projectVersion)) {
projectVersion = projectVersion + "-SNAPSHOT";
Copy link
Owner

Choose a reason for hiding this comment

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

There is Artifact.SNAPSHOT_VERSION constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I change it.

@@ -177,9 +177,23 @@ public void execute() throws MojoExecutionException, MojoFailureException {
gitMergeNoff(hotfixBranchName);

final String currentVersion = getCurrentProjectVersion();
if (useSnapshotInHotfix && ArtifactUtils.isSnapshot(currentVersion)) {
Copy link
Owner

Choose a reason for hiding this comment

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

Will it work with support branches and release branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This feature work with hotfix and release branches. This feature is not implemented for support branches. Do you think that this should be implemented?

Copy link
Owner

Choose a reason for hiding this comment

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

hotfix will merge into the release branch if the release branch exists. The hotfix of support branch is also a thing. :)

Will it work with them?

@@ -223,9 +223,23 @@ public void execute() throws MojoExecutionException, MojoFailureException {
// get current project version from pom
final String currentVersion = getCurrentProjectVersion();

if ((commitDevelopmentVersionAtStart && useSnapshotInRelease) && ArtifactUtils.isSnapshot(currentVersion)) {
Copy link
Owner

Choose a reason for hiding this comment

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

This creates additional commit in production branch with snapshot version. Production branch should be clean, meaning each commit is deployable, meaning no snapshot version in production.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this block of code I'm removing the snapshot suffix when we are using useSnapshotInRelease and the version of artifact is snapshot.
The commit executed over the production branch is clean.

Copy link
Owner

Choose a reason for hiding this comment

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

But this commit is in the production branch.

@aleksandr-m aleksandr-m merged commit 9715c26 into aleksandr-m:master Jun 17, 2018
@aleksandr-m
Copy link
Owner

@alsanrum PR merged. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants