-
Notifications
You must be signed in to change notification settings - Fork 69
Add the tag name separator option (between ArtifactId and Version) #92
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
base: master
Are you sure you want to change the base?
Add the tag name separator option (between ArtifactId and Version) #92
Conversation
(cherry picked from commit a293c91a09fe7f27e33027554cbab9f2b7464bf3)
| .buildFromCurrentDir(); | ||
| ResolverWrapper resolverWrapper = new ResolverWrapper(factory, artifactResolver, remoteRepositories, localRepository); | ||
| Reactor reactor = Reactor.fromProjects(log, repo, project, projects, buildNumber, modulesToForceRelease, noChangesAction, resolverWrapper, versionNamer); | ||
| Reactor reactor = Reactor.fromProjects(log, repo, project, projects, buildNumber, modulesToForceRelease, noChangesAction, resolverWrapper, versionNamer, tagNameSeparator); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not ideal to make an excessively long list of parameters even longer. I have an upcoming PR that fixes this and I would appreciated if you could hold back this PR until then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, first come first served ! As soon as your PR is approved and merged, il will rebase and adapt mine.
| public String arguments; | ||
|
|
||
| /** | ||
| * <p>Determines the separator between artifactId and groupId<p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this documentation needs more detail about what this is for and what tags look like, which is {artifactid}-{version} (doesn't actually have groupid). Maybe this should actually be a string like {artifactid}-{version} and the code just does a string replace on {artifactid} and {version}. There could be {groupid} too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I will try to do that. That's what i will implement :
if my custom tag name parameter contains at least substrings "{artifactId}" and "{version}" (minimum required to guarantee uniqueness of a git tag) then do a
tagName=customTagName
.replace("{artifactId}", project.getArtifactId())
.replace("{groupId}", project.getGroupId())
.replace("{version}", version.releaseVersion());else log a warn and use default value {artifactid}-{version}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait, would that complicate the tag lookups in the part of the code that guesses the next build number? (It's been a while since I worked on this so can't really remember)
| import static scaffolding.GitMatchers.hasTag; | ||
| import static scaffolding.MvnRunner.assertArtifactInLocalRepo; | ||
|
|
||
| public class InheritedVersionsWithTagNameSeparatorTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test should be more focused on the actual change. So delete the copied tests around inheritance and rollbacks etc and just test that the tag separator logic is working. Include tests for invalid separators (i.e. things that are not allowed in git tags).
Hello Mr Flower,
Thanks for your plugin, it's really helpful for our software factory.
I needed to amend it a little bit, to classify my module tags into directories in git. So i added this option in the code. Set by default as "-" to avoid regressions as possible as it could be. (mvn tests ok by the way)
I wonder if you could take a look at it anytime.
regards
Thomas Bonset