FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(bump): remove `NotAllowed` related to `--get-next` option, and other related refactoring by bearomorphism · Pull Request #1645 · commitizen-tools/commitizen · GitHub

/ commitizen Public

fix(bump): remove NotAllowed related to --get-next option, and other related refactoring - #1645

Merged
Lee-W merged 1 commit into
commitizen-tools:v4-10-1from
bearomorphism:dev-bump
Nov 19, 2025
ConversationCommits (1)ChecksFiles changed
Merged

fix(bump): remove NotAllowed related to --get-next option, and other related refactoring#1645
Lee-W merged 1 commit into
commitizen-tools:v4-10-1from
bearomorphism:dev-bump

Conversation

bearomorphism commented Nov 10, 2025

Copy link
Copy Markdown
Collaborator

Changes

  1. Remove unreachable logic, such as setting dry_run = True while we already have get_next = True. The latter condition raises a GetNextExit() exception in the original code, so no effect if we set dry_run to True. Moreover, the original code is self.dry_run = True which is ineffective since we never use the member variable.
  2. Simplify things.
  3. Rename the variable get_next to next_version_to_stdout. The variable name get_next is very unclear and hard to guess what it will do. I think we can consider renaming the option in the next major release.
  4. (relate Not possible to use --get-next when changelog is enabled #1640) Replace NotAllowed related to --get-next option with warnings.
  5. Remove GetNextExit exception because we already have DryRunExit. --get-next is just --dry-run but doing less things IMO.

bearomorphism mentioned this pull request Nov 10, 2025
Closed

bearomorphism commented Nov 10, 2025

Copy link
Copy Markdown
Collaborator Author

Is the branck v4-10-1 broken?

woile approved these changes Nov 10, 2025
View reviewed changes

woile left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

Lee-W commented Nov 11, 2025

Copy link
Copy Markdown
Member

I just update the v4-10-1 branch. Could you please rebase from it again? We might need to come up with another release strategy, but this is what we have for the time being

Lee-W marked this pull request as draft November 11, 2025 01:50
Lee-W added this to the 4.10.1 milestone Nov 11, 2025

codecov Bot commented Nov 11, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (v4-10-1@9f3ec86). Learn more about missing BASE report.

Files with missing lines Patch % Lines
commitizen/commands/bump.py 87.50% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             v4-10-1    #1645   +/-   ##
==========================================
  Coverage           ?   98.64%           
==========================================
  Files              ?       60           
  Lines              ?     2651           
  Branches           ?        0           
==========================================
  Hits               ?     2615           
  Misses             ?       36           
  Partials           ?        0
Flag Coverage Δ
unittests 98.64% <87.50%> (?)

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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

bearomorphism marked this pull request as ready for review November 11, 2025 13:26

bearomorphism commented Nov 11, 2025

Copy link
Copy Markdown
Collaborator Author

@Lee-W @noirbizarre please review

bearomorphism commented Nov 18, 2025

Copy link
Copy Markdown
Collaborator Author

We might need to come up with another release strategy

We may prioritize this. Clarifying which commit new branches should be based on would make it much easier for new contributors, and it should also help reduce merge conflicts and PRs being accidentally closed due to test branch deletion.

Lee-W approved these changes Nov 19, 2025
View reviewed changes

Lee-W left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

One nit. Close to merge

Comment thread commitizen/commands/bump.py

Lee-W commented Nov 19, 2025

Copy link
Copy Markdown
Member

We might need to come up with another release strategy

We may prioritize this. Clarifying which commit new branches should be based on would make it much easier for new contributors, and it should also help reduce merge conflicts and PRs being accidentally closed due to test branch deletion.

Generally feat goes to next minor release and others goes to next patch release. docs, tests can go the master directly

Lee-W added pr-status: ready-to-merge almost ready to merge. just keep it for a few days for others to check and removed pr-status: wait-for-modification pr-status: wait-for-review labels Nov 19, 2025
Lee-W merged commit 280bea0 into commitizen-tools:v4-10-1 Nov 19, 2025
18 checks passed
bearomorphism deleted the dev-bump branch November 19, 2025 06:21
bearomorphism mentioned this pull request May 10, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Lee-W Lee-W approved these changes
woile woile approved these changes
noirbizarre Awaiting requested review from noirbizarre

Assignees

No one assigned

Labels

pr-status: ready-to-merge almost ready to merge. just keep it for a few days for others to check type: bug type: refactor

Projects

None yet

Milestone

4.10.1

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL