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 --exact option to bump, to force it to honor the increment for prereleases #981

Merged
merged 4 commits into from
Feb 26, 2024

Conversation

chadrik
Copy link
Contributor

@chadrik chadrik commented Feb 5, 2024

Description

PR #799 just merged which adds some nuance to prerelease bumping by improving the default logic for when to bump major versions and prerelease suffixes. While this is an improvement over the original logic, it's still very prescriptive and I think there's room for debate over whether this is a one-size-fits-all solution. There are scenarios where more control is required.

This PR adds bump --exact to honor the increment when a prerelease suffix is present, which provides the user with more control rather utilizing the current approach with its various edge cases.

I made this change in support of a gitflow-on-autopilot workflow (demo here) using the common set of 3 branches: develop, staging, master. In this autopilot workflow, when a sprint ends and it's time to create a new release, all of the branches merge to the right (developstagingmaster), and develop is restarted for the next round of beta development. When restarting beta, I want cz bump --increment MINOR --prelease beta to increment to 1.0.0b5 → 1.1.0b0

Here's a sample git graph:

image

Checklist

  • Add test cases to all the changes you introduce
  • Run ./scripts/format and ./scripts/test locally to ensure this change passes linter check and test
  • Test the changes on the local machine manually
  • Update the documentation for the changes

Expected behavior

These tests cases compare the behavior with and without exact:

exact_cases = [
    (("1.0.0", "PATCH", None, 0, None), "1.0.1"),
    (("1.0.0", "MINOR", None, 0, None), "1.1.0"),
    # with excact_mode=False: "1.0.0b0"
    (("1.0.0a1", "PATCH", "beta", 0, None), "1.0.1b0"),
    # with excact_mode=False: "1.0.0b1"
    (("1.0.0b0", "PATCH", "beta", 0, None), "1.0.1b0"),
    # with excact_mode=False: "1.0.0rc0"
    (("1.0.0b1", "PATCH", "rc", 0, None), "1.0.1rc0"),
    # with excact_mode=False: "1.0.0-rc1"
    (("1.0.0rc0", "PATCH", "rc", 0, None), "1.0.1rc0"),
    # with excact_mode=False: "1.0.0rc1-dev1"
    (("1.0.0rc0", "PATCH", "rc", 0, 1), "1.0.1rc0.dev1"),
    # with excact_mode=False: "1.0.0b0"
    (("1.0.0a1", "MINOR", "beta", 0, None), "1.1.0b0"),
    # with excact_mode=False: "1.0.0b1"
    (("1.0.0b0", "MINOR", "beta", 0, None), "1.1.0b0"),
    # with excact_mode=False: "1.0.0rc0"
    (("1.0.0b1", "MINOR", "rc", 0, None), "1.1.0rc0"),
    # with excact_mode=False: "1.0.0rc1"
    (("1.0.0rc0", "MINOR", "rc", 0, None), "1.1.0rc0"),
    # with excact_mode=False: "1.0.0rc1-dev1"
    (("1.0.0rc0", "MINOR", "rc", 0, 1), "1.1.0rc0.dev1"),
]

Steps to Test This Pull Request

Additional context

@chadrik
Copy link
Contributor Author

chadrik commented Feb 5, 2024

For the record, I'm thinking of wrapping up some of the functionality from my gitlfow autopilot demo into a commitizen-gitflow project, which would extend commitizen with knowledge about branch names, merge order, and their relationship to prerelease suffixes. Once it's done, if you like it, I could make a PR here to add the support directly to commitizen.

Copy link

codecov bot commented Feb 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (120d514) 97.33% compared to head (4d666cb) 97.44%.
Report is 182 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #981      +/-   ##
==========================================
+ Coverage   97.33%   97.44%   +0.11%     
==========================================
  Files          42       55      +13     
  Lines        2104     2392     +288     
==========================================
+ Hits         2048     2331     +283     
- Misses         56       61       +5     
Flag Coverage Δ
unittests 97.44% <100.00%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@woile
Copy link
Member

woile commented Feb 6, 2024

Hey, I'll have to read this in detail once I'm no longer sick. I would like to hear the thoughts from @noirbizarre @Lee-W @jenstroeger

@Lee-W
Copy link
Member

Lee-W commented Feb 6, 2024

We have a longer holiday in the following week. I think I'll have some time to check

Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

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

Hi @chadrik , thanks for your great contribution! I'm good with the overall implementation. But I'm not sure whether we should name this argument as exact. I'm not able to understand it even after reading it for a while. Would it be possible for us to come up with a more descriptive name? @woile @noirbizarre would appreciate if you have some idea on this one.

Also we might need some documentation and examples for this chanhge

"action": "store_true",
"help": (
"treat the increment and prerelease arguments "
"explicitly. Disables logic that attempts to deduce "
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"explicitly. Disables logic that attempts to deduce "
"explicitly. Disables logic that attempts to deduce "

@chadrik
Copy link
Contributor Author

chadrik commented Feb 16, 2024

Hi @chadrik , thanks for your great contribution! I'm good with the overall implementation. But I'm not sure whether we should name this argument as exact. I'm not able to understand it even after reading it for a while. Would it be possible for us to come up with a more descriptive name? @woile @noirbizarre would appreciate if you have some idea on this one.

What this flag does is apply the exact changes that have been specified (or determined from the commit log). For example, --prerelease beta will always result in a b tag, --increment PATCH (or the inferred equivalent from commit inspection) will always increase the patch component. These things are not true in the normal mode, which attempts to guess the best next version based on typical version progression.

Here are some new examples that I think are illustrative of the new behavior:

cz bump --increment=MAJOR
without --exact   2.0.0a0 --> 2.0.0
with --exact      2.0.0a0 --> 3.0.0

cz bump --increment=MINOR
without --exact   2.0.0a0 --> 2.0.0
with --exact      2.0.0a0 --> 2.1.0

cz bump --increment=PATCH
without --exact   2.0.0a0 --> 2.0.0
with --exact      2.0.0a0 --> 2.0.1

cz bump --increment=MINOR --prerelease=alpha
without --exact   2.0.0b0 --> 2.0.0b1
with --exact      2.0.0b0 --> 2.1.0a0

cz bump --increment=PATCH --prerelease=alpha
without --exact   2.0.0b0 --> 2.0.0b1
with --exact      2.0.0b0 --> 2.0.1a0

cz bump --increment=MAJOR --prerelease=alpha   (same)
without --exact   1.0.0 --> 2.0.0a0
with --exact      1.0.0 --> 2.0.0a0

cz bump --increment=MINOR --prerelease=alpha
without --exact   2.0.0a0 --> 2.0.0b1
with --exact      2.0.0a0 --> 2.1.0a0

cz bump --increment=PATCH --prerelease=alpha
without --exact   2.0.0a1 --> 2.0.0b1
with --exact      2.0.0a1 --> 2.0.1a0

The second to last example is particularly interesting. without --exact we don't get anything close to what was requested: the increment and the prerelease tag are overridden. This is fine when you want something that does the "right thing" most of the time, but I need greater control in my CI/CD workflows : demo here

Here are some other potential names:

  1. --exact-increment
  2. --use-exact-increment
  3. --explicit-increment
  4. --no-auto-increment
  5. --no-auto-progression
  6. --no-inferred-version

I'm open to ideas!

@Lee-W
Copy link
Member

Lee-W commented Feb 18, 2024

Hi @chadrik , thanks for your great contribution! I'm good with the overall implementation. But I'm not sure whether we should name this argument as exact. I'm not able to understand it even after reading it for a while. Would it be possible for us to come up with a more descriptive name? @woile @noirbizarre would appreciate if you have some idea on this one.

What this flag does is apply the exact changes that have been specified (or determined from the commit log). For example, --prerelease beta will always result in a b tag, --increment PATCH (or the inferred equivalent from commit inspection) will always increase the patch component. These things are not true in the normal mode, which attempts to guess the best next version based on typical version progression.

Here are some new examples that I think are illustrative of the new behavior:

cz bump --increment=MAJOR
without --exact   2.0.0a0 --> 2.0.0
with --exact      2.0.0a0 --> 3.0.0

cz bump --increment=MINOR
without --exact   2.0.0a0 --> 2.0.0
with --exact      2.0.0a0 --> 2.1.0

cz bump --increment=PATCH
without --exact   2.0.0a0 --> 2.0.0
with --exact      2.0.0a0 --> 2.0.1

cz bump --increment=MINOR --prerelease=alpha
without --exact   2.0.0b0 --> 2.0.0b1
with --exact      2.0.0b0 --> 2.1.0a0

cz bump --increment=PATCH --prerelease=alpha
without --exact   2.0.0b0 --> 2.0.0b1
with --exact      2.0.0b0 --> 2.0.1a0

cz bump --increment=MAJOR --prerelease=alpha   (same)
without --exact   1.0.0 --> 2.0.0a0
with --exact      1.0.0 --> 2.0.0a0

cz bump --increment=MINOR --prerelease=alpha
without --exact   2.0.0a0 --> 2.0.0b1
with --exact      2.0.0a0 --> 2.1.0a0

cz bump --increment=PATCH --prerelease=alpha
without --exact   2.0.0a1 --> 2.0.0b1
with --exact      2.0.0a1 --> 2.0.1a0

The second to last example is particularly interesting. without --exact we don't get anything close to what was requested: the increment and the prerelease tag are overridden. This is fine when you want something that does the "right thing" most of the time, but I need greater control in my CI/CD workflows : demo here

Here are some other potential names:

1. `--exact-increment`

2. `--use-exact-increment`

3. `--explicit-increment`

4. `--no-auto-increment`

5. `--no-auto-progression`

6. `--no-inferred-version`

I'm open to ideas!

This is helpful! It would be great if we could add these examples somewhere in docs. Regarding the args option, I like --exact-increment. Also, I noticed there's a conflict in this PR. We might need your help to resolve it

@noirbizarre
Copy link
Member

Sorry, I'm on a trip without computer and with very limited connectivity. I'll be able to review it next week.
@Lee-W if you feel this is okay and ready to merge now, go on. If not, I'll review it ASAP when I come back

@noirbizarre
Copy link
Member

Forgot to say, this seems helpful and I'm totally okay with it.
Note: it might be interesting to also have this option available in the github action

@chadrik
Copy link
Contributor Author

chadrik commented Feb 18, 2024

I made some updates:

  • Tests added
  • Docs added
  • Name changed to --exact-increment

When bumping a prerelease to a new prerelease, honor the detected increment
and preserve the prerelease suffix, rather than bumping to the next
non-prerelease version
This avoids calls to git and additional validations that are not necessary when using --increment
This provides some future proofing for implementing new version progression behaviors
@chadrik
Copy link
Contributor Author

chadrik commented Feb 20, 2024

Note: I changed the --exact-increment bool flag to --increment-mode enum flag with two options: "linear" (default) and "exact".

This provides some future proofing for implementing new version progression behaviors, since this is a subjective area with plenty of room for valid and differing opinions. For example, if someone creates an issue complaining about how the linear progression change introduced in #799 broke backward compatibility for them, it would be possible to implement a "legacy" mode. I could also foresee a "gitflow" mode that determines --prerelease from the current branch name (eg. develop --> beta, release --> rc).

@chadrik
Copy link
Contributor Author

chadrik commented Feb 23, 2024

nudge

Copy link
Member

@woile woile left a comment

Choose a reason for hiding this comment

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

Really good idea, I like the new naming, and the docs. Code looks good.
I'll merge Monday if there are no other comments 👍🏻

@Lee-W
Copy link
Member

Lee-W commented Feb 26, 2024

Really good idea, I like the new naming, and the docs. Code looks good. I'll merge Monday if there are no other comments 👍🏻

Sounds good! I'm traveling these days, but i've checked before the minor fixes. I think I'm good with merging it as well!

@woile woile merged commit f9a0e2b into commitizen-tools:master Feb 26, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants