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

Add command line input for tag management #185

Merged
merged 1 commit into from
Mar 8, 2017
Merged

Conversation

andreaTP
Copy link
Contributor

@andreaTP andreaTP commented Feb 1, 2017

Added command line arguments to pass default parameter on tagRelease Step.

@andreaTP
Copy link
Contributor Author

maybe @jroper @gseitz or anybody else can comment here?

@jroper
Copy link
Member

jroper commented Feb 17, 2017

I'm not 100% sure of what the use case is here, nor do I 100% understand how the tag-default attribute is used, could you explain a little more?

Copy link
Member

@jroper jroper left a comment

Choose a reason for hiding this comment

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

It would be good to see this option documented in the README.


> release with-defaults tag-default o
> release with-defaults tag-default k
> release with-defaults tag-default v0.1.0-debug
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand this scripted tests - why does release with defaults fail and release with-defaults tag-default a also fail, but release with-defaults tag-default o doesn't fail? What's so special about o that makes it different to a? I just don't get it. Could you add some comments here to explain what's being tested and what's going on?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

going to add comments around.

@@ -136,17 +137,21 @@ object ReleasePlugin extends AutoPlugin {
(Space ~> token("release-version") ~> Space ~> token(StringBasic, "<release version>")) map ParseResult.ReleaseVersion
private[this] val NextVersion: Parser[ParseResult] =
(Space ~> token("next-version") ~> Space ~> token(StringBasic, "<next version>")) map ParseResult.NextVersion
private[this] val TagDefault: Parser[ParseResult] =
(Space ~> token("tag-default") ~> Space ~> token(StringBasic, "<next version>")) map ParseResult.TagDefault
Copy link
Member

Choose a reason for hiding this comment

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

-      (Space ~> token("tag-default") ~> Space ~> token(StringBasic, "<next version>")) map ParseResult.TagDefault
+      (Space ~> token("tag-default") ~> Space ~> token(StringBasic, "<tag default>")) map ParseResult.TagDefault

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right thanks

@andreaTP
Copy link
Contributor Author

TL;DR
Just let user adjust hardcoded default of the tagging step

Rationale
The usage of this parameter is explicitly for usage on CI, release with-default alone doesn't let me decide the rules for tagging explicitly, and the only default is the hardcoded abort.
With this PR you will be able to pass through command line arguments different tagging default.
My specific use case is that I want to overwrite SNAPSHOT tags on git, but this enable also the user to give personal tags through the argument in CI.

@andreaTP
Copy link
Contributor Author

done fixes and addressed comments.
rebased modifications.

Thanks for taking care and for comments :-)

@andreaTP
Copy link
Contributor Author

CI failure looks spurious to me

@jroper
Copy link
Member

jroper commented Mar 7, 2017

Ah, I get it now. Perhaps then a more descriptive name would be default-tag-exists-answer. Also, the description of the value used in the parser should probably be o|k|a, rather than <tag-default>.

Also, this needs to be documented in the README, eg, in the section about with-defaults, saying that the answer to the existing VCS tag prompt can be overridden using the default-tag-exists-answer argument.

@andreaTP
Copy link
Contributor Author

andreaTP commented Mar 7, 2017

Done, thanks a lot for the feedback! :-)

@jroper jroper merged commit d0f7c29 into sbt:master Mar 8, 2017
@andreaTP
Copy link
Contributor Author

andreaTP commented Mar 8, 2017

Thanks!

@andreaTP
Copy link
Contributor Author

andreaTP commented Mar 8, 2017

Due to recent updates, could you please publish a release?
Thanks in advance

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.

3 participants