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

feat: preserve prerelease linearity #800

Conversation

eduardocardoso
Copy link
Contributor

Description

Forbid going back in prerelease level, after a beta or rc prerelease is made the next prerelease without a version number bump will be the same type if a lower or equal level is provided.
a -> b -> rc

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

After a prerelease it will only allow to keep or increase the prerelease level.
A bump from b0 with --prerelease alpha will generata a b1 release instead of going back to a0.

Steps to Test This Pull Request

Start at versions 0.1.0

  1. git commit --allow-empty -m 'fix: bug'
  2. cz bump --prerelease alpha → bumps to 0.1.1a0
  3. cz bump --prerelease beta → bumps to 0.1.1b0
  4. cz bump --prerelease alpha → bumps to 0.1.1b1
  5. cz bump --prerelease rc → bumps to 0.1.1rc0
  6. cz bump --prerelease alpha → bumps to 0.1.1rc1
  7. cz bump --prerelease beta → bumps to 0.1.1rc2

Additional context

@codecov
Copy link

codecov bot commented Jul 27, 2023

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Files Coverage Δ
commitizen/__version__.py 100.00% <100.00%> (ø)
commitizen/bump.py 100.00% <100.00%> (ø)
commitizen/changelog.py 99.39% <100.00%> (-0.11%) ⬇️
commitizen/changelog_formats/asciidoc.py 100.00% <100.00%> (ø)
commitizen/changelog_formats/base.py 100.00% <100.00%> (ø)
commitizen/changelog_formats/markdown.py 100.00% <100.00%> (ø)
commitizen/changelog_formats/restructuredtext.py 100.00% <100.00%> (ø)
commitizen/changelog_formats/textile.py 100.00% <100.00%> (ø)
commitizen/commands/bump.py 97.72% <100.00%> (+0.08%) ⬆️
commitizen/commands/changelog.py 99.07% <100.00%> (+0.13%) ⬆️
... and 25 more

📢 Thoughts on this report? Let us know!.

@Lee-W
Copy link
Member

Lee-W commented Jul 29, 2023

Step 4 might be confusing. The logic makes sense. But we might need to design it in another way. Adding a --force-increase flag might make more sense? @woile @noirbizarre Would love to see what you think 🙂

@woile
Copy link
Member

woile commented Jul 30, 2023

I'm not a big fan of this change, because things start getting confusing. I'd rather raise an error of the wrong value is used. But, I also struggle to see if it's worth to enforce it. Maybe behind a setting "enforce_prereleases"

What's the motivation for this PR?

@jenstroeger
Copy link
Contributor

What's the motivation for this PR?

This PR builds on top of PR #799, and came from our branching model at work: PR branches merge into “downstream” branches and release packages on each branch. So,

  • the development branch (publishes pre-release alpha) merges into
  • the testing branch (publishes pre-release beta) which merges into
  • the staging branch (publishes pre-release rc) which merges into
  • the main branch (publishes the final release).

Each of these downstream merges automatically creates a bump commit on the target branch and a tag on the bump commit. However, to ensure a consistent history across branches, upstream branches always automatically rebase on top of downstream branches. I illustrated @eduardocardoso’s example above:

cz-800

On the right side of the image you have the growing git log history where you can see the bump: commits; that history is the same on all branches.

Because we continuously rebase downstream branches, we quickly get into a place where the development branch has a bump: 0.1.1rc0 commit (including tag) — the next commit to development must bump that rc0 to preserve linearity: it can not bump below rc0. Does that make sense?

@woile
Copy link
Member

woile commented Nov 3, 2023

I see 🤔 I'm fine with it as long as proper documentation is added, I can already foresee users complaining that doing cz bump --prerelease alpha → bumps to 0.1.1b1 raises the wrong prerelease, and I won't know why 😆

I think we are taking an opinionated approach on how it should be done, and that's what I would like to see reflected in the docs. I'm not a big user of prereleases myself, so I trust you on this model.

@jenstroeger
Copy link
Contributor

jenstroeger commented Nov 3, 2023

Agreed — and it probably makes sense to add a command-line switch that enables precedence bumping. For example if the current version is 0.1.1b0 then

  • cz bump --prerelease beta bumps to 0.1.1b1
  • cz bump --prerelease alpha is an error because a beta tag was expected
  • cz bump --prerelease (no argument!) bumps to to 0.1.1b1 or
  • cz bump --prerelease alpha --consider-precedence also bumps to 0.1.1b1

We can either make the argument to --prerelease optional in which case it detects and bumps based on the tag with the highest precedence, or we add an argument that explicitly enables bumping based on the tag with the highest precedence. (See also our previous discussions in issues #688 and conventional-commits/conventionalcommits.org#398.)

Also note that this PR addresses the uncertainty of this test:

# this cases should be handled gracefully
unexpected_cases = [
(("0.1.1rc0", None, "alpha", 0, None), "0.1.1a0"),
(("0.1.1b1", None, "alpha", 0, None), "0.1.1a0"),
]

Instead of expecting “graceful” handling of this case, we can now clearly define the expected behavior.

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.

LGTM

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.

Please add a new section about pre-releases inside the bump command explaining "prerelease linearity" 🙏🏻

@jenstroeger
Copy link
Contributor

We can close this PR: I have merged it into PR #799 with commit b9e768e.

@Lee-W Lee-W closed this Dec 18, 2023
@Lee-W
Copy link
Member

Lee-W commented Dec 18, 2023

Close as suggested

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