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

Mechanism to configure a pre-release-weight for the tagged commits #2222

Conversation

ruhullahshah
Copy link
Contributor

Background

This PR is motivated by issue #1994. As a user, I should be able to:

  • Specify by means of a regex in the configuration as to how GitVersion should construct semantic version numbers from the Git tags.
  • Specify weights for pre-release labels in the Git tags.

Proposal

In order to achieve the stated objectives, we can extend the global configuration as:

tag-config: 
    file-versioning-format: "v{Major}.{Minor}.{Patch}.{TagWeightedPreReleaseNumber}"
    pre-release-label-weights:
       alpha: 10000
       beta: 20000

A new GitVersion variable TagWeightedPreReleaseNumber will be introduced which can have similar semantics as the existing WeightedPreReleaseNumber, i.e. it can be computed as pre-release-label-weight + CommitsSinceVersionSource.

The file-versioning-format will have similar semantics as assembly-file-versioning-format and can specify environment variables as well.

@ruhullahshah
Copy link
Contributor Author

@jbaehr @asbjornu

I have created an initial proposal for addressing issue #1994 and added a failing unit test for the type TaggedCommitVersionStrategy. I have not set up the config for the unit test yet. Once the config is finalised, I will proceed with the feature construction and add appropriate config options to the unit test and to some integration tests. Please let me know your thoughts about the proposal.

@jbaehr
Copy link
Contributor

jbaehr commented May 6, 2020

While don't fully understand the use cases of #1994 and #1600 (and, to be honest, I haven't dug deeply into the later), I can't really comment on the tag-config proposal. Apart from finding an additional, nested file-versioning-format a bit odd; What does it offer opposed to the global assembly-file-versioning-format with the standard WeightedPreReleaseNumber?

Anyway, I propose to go for a simpler approach first, just enough for the vanilla gitflow use case, where you only tag final versions. Simply defining a static default pre-release weight for tags should be enough for preventing the WeightedPreReleaseNumber to be null on builds from tagged commits. This default should be as high as possible, as there is no semver "larger" than the final build. And "commits since version source" should be always zero, per definition, else your current commit is not the tagged commit, ergo a branch config should apply. For Windows we have 16 bit for this, thus $2^16 - 1$ would give a good value. Or 60000, for those preferring human-readable numbers (programmers are not "normal" humans).

So either we can go for a new top-level config key, a simple scalar. Or, in the same spirit as the branch config something like

tags:
  default:
    pre-release-weight: 60000

Later, this can still be extend with different named tag configs, with regex matching and extracted pre-release labels, what ever...

@asbjornu
Copy link
Member

asbjornu commented May 8, 2020

If your proposed solution is sufficient, it's definitely the simplest one to implement, maintain and document, @jbaehr. Thoughts, @ruhullahshah?

@ruhullahshah
Copy link
Contributor Author

@jbaehr, @asbjornu

  • My proposal was aimed to be a more general solution, with GitFlow being a special case, wherein under pre-release-label-weights one could specify default: 60000 for the final release build.
  • file-versioning-format: "v{Major}.{Minor}.{Patch}.{TagWeightedPreReleaseNumber}" was aimed at instructing the TaggedCommitVersionStrategy as to how to construct the version numbers out of tags, for instance 1.0.0-alpha would get mapped to 1.0.0.10000
  • However, I see @jbaehr 's point and it indeed simplifies the implementation if we are targeting GitFlow only for now. Also, it serves as a good starting point, as already mentioned by @jbaehr, for generalizing the implementation.
  • I will take up the proposal made by @jbaehr and implement it.

@ruhullahshah ruhullahshah changed the title [WIP] Introduce regex support for Git tags and provide mechanism to translate pre-release labels in the the tags to numerical weights [WIP] Mechanism to configure a pre-release-weight for the tags May 8, 2020
@asbjornu
Copy link
Member

asbjornu commented May 8, 2020

Sounds good, @ruhullahshah! Looking forward to seeing the implementation.

@ruhullahshah ruhullahshah changed the title [WIP] Mechanism to configure a pre-release-weight for the tags Mechanism to configure a pre-release-weight for the tagged commits May 9, 2020
@ruhullahshah
Copy link
Contributor Author

@asbjornu I have added the changes that I believe are minimal and necessary to address the issues described by users in #1994.

@asbjornu
Copy link
Member

asbjornu commented May 10, 2020

Sounds and looks great, @ruhullahshah! Could you give this a quick review too, @jbaehr? A rebase on the HEAD of master would probably also be a good idea. :)

for translating the pre-release labels in Git tags to weights.
…, especially

in GitFlow branching scenarios. This could lead to an emission of
AssemblyFileSemVer that is not strictly increasing.

Solution: Introduce a new configuration parameter,
tag-pre-release-weight, that could overrwrite the
WeightedPreReleaseNumber if it turns out to be 0.
@ruhullahshah ruhullahshah force-pushed the feature/1994_PreReleaseWeightAndRegexForTags branch from b4a416c to 80473c5 Compare May 11, 2020 13:05
@jbaehr
Copy link
Contributor

jbaehr commented May 11, 2020

@ruhullahshah thanks for your work!

I only question the "0" default for the tag-pre-release-weight. In my eyes (as motivated in my first comment) the value should be high, to make sense. As the WeightedPreReleaseNumber was previously undefined/empty/null, we don't need zero for backward compatibility, either. Instead, the default here should fit with the defaults from the branch configurations, see #1433, i.e. 60_000.

@ruhullahshah
Copy link
Contributor Author

@jbaehr You are welcome.

I agree, a default weight of 60000 makes more sense. That way a user using the GitFlow branching model might not even need to specify the value in the configuration. I have updated the PR for the same.

@asbjornu Could we ship this now, do not want to rebase anymore :P ?

@asbjornu asbjornu merged commit 75f4774 into GitTools:master May 11, 2020
@asbjornu asbjornu added this to the 5.3.x milestone May 11, 2020
@asbjornu
Copy link
Member

Thank you so much for your work on this @ruhullahshah, and for your valuable input @jbaehr! 🙏

arturcic pushed a commit that referenced this pull request May 11, 2020
Merge pull request #2222 from ruhullahshah/feature/1994_PreReleaseWeightAndRegexForTags

Mechanism to configure a pre-release-weight for the tagged commits
@ruhullahshah
Copy link
Contributor Author

@asbjornu You are welcome!

@ruhullahshah ruhullahshah deleted the feature/1994_PreReleaseWeightAndRegexForTags branch May 12, 2020 05:39
@arturcic arturcic modified the milestones: 5.3.x, 5.3.3 May 12, 2020
@arturcic arturcic linked an issue May 12, 2020 that may be closed by this pull request
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.

WeightedPreReleaseNumber null on tag
4 participants