-
Notifications
You must be signed in to change notification settings - Fork 1
Conversation
0be79f3
to
4f29a1d
Compare
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.
The implementatio looks good to me.
I would like to address just the smaller stuff. I also saw some missing newlines in last line on some files :)
On top of that could you please also mark in the PR description what the purpose of the new GitVersion, GitVersionRepository is for. We talked about it and I understand from the code that you created that to move some code that was repeated in there.
As a last also mark the removal of the marker strategy. It looks like you renamed it that it becomes the prefix strategy. It kind of is true. Just that the usage was different.
@@ -83,6 +83,7 @@ class VersionPlugin implements Plugin<Project> { | |||
def rawValue = VersionPluginConventions.versionCodeOffset.getValue(project) | |||
Integer.parseInt(rawValue.toString()) | |||
8000 })) | |||
extension | |||
extension.prefix.convention("v") |
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.
We don't want to implement our usual fallback to env/property flow? I see no harm of allowing the prefix to be set via environment.
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 is addressed in #60. I tried to make this PR fit the same code style as the current one in atlas version to minimize the changes. Then later I change the style for everyone.
*/ | ||
TagStrategy getTagStrategy() { | ||
tagStrategy | ||
Property<String> prefix = objects.property(String) |
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.
You should use the same template @Azurelol and I use for setting up gradle properties.
In case of an extension you can just use a groovy public field. But in case of tasks gradle wants the @input to be put on a getter and that the field is marked private. Since we use traits in a lot of places to share these properties and implementations etc we tend to use the same setup everywhere.
private final Property<$TYPE$> $NAME$ = objects.property($TYPE$)
$INPUT_TYPE$
Property<$TYPE$> get$U_NAME$() {
$NAME$
}
void set$U_NAME$(Provider<$TYPE$> value) {
$NAME$.set(value)
}
void set$U_NAME$($TYPE$ value) {
$NAME$.set(value)
}
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.
Also addressed in #60. I have my own template for this as well, I guess we should put stuff together and make some shared templates for the team?
@@ -309,6 +315,10 @@ trait VersionPluginExtension implements BaseSpec { | |||
releaseStage | |||
} | |||
|
|||
Provider<GitVersionRepository> versionRepo = git.map { | |||
Grgit it -> GitVersionRepository.fromTagStrategy(it, new PrefixTagStrategy(prefix.get(), true)) |
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.
can we also please use the same style for closueres etc. The parameters Grgit it ->
should be on the line before if you need to split the closure in multiple lines.
Description
Adds new property
prefix
toVersionPluginExtension
that allows the user to edit a which prefix will be used to search for its applications versions. Before, this prefix was hardcoded to "v".To do so, version-related git operations were centralised in
GitVersionRepository
. Before they were spread around and duplicated in several different parts of the code. Tag filtering is still done byTagStrategy
-derived classes, which are now associated withGitVersionRepository
.This is a breaking change because now the TagStrategy used to generate version codes are now consistent with the tag strategy. used to generate the actual version. Beforehand, the version code TagStrategy was hardcoded, while the version TagStrategy was variable.
Changes