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

fix(get-next-bump): fix to permit usage of --get-next option even when update_changelog_on_bump is set to true #1301

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 22 additions & 12 deletions commitizen/commands/bump.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,8 @@ def __init__(self, config: BaseConfig, arguments: dict):
},
}
self.cz = factory.commiter_factory(self.config)
self.changelog = arguments["changelog"] or self.config.settings.get(
"update_changelog_on_bump"
)
self.changelog_flag = arguments["changelog"]
self.changelog_config = self.config.settings.get("update_changelog_on_bump")
self.changelog_to_stdout = arguments["changelog_to_stdout"]
self.git_output_to_stderr = arguments["git_output_to_stderr"]
self.no_verify = arguments["no_verify"]
Expand Down Expand Up @@ -207,17 +206,24 @@ def __call__(self) -> None: # noqa: C901
"--local-version cannot be combined with --build-metadata"
)

# If user specified changelog_to_stdout, they probably want the
# changelog to be generated as well, this is the most intuitive solution
self.changelog = self.changelog or bool(self.changelog_to_stdout)

if get_next:
if self.changelog:
# if trying to use --get-next, we should not allow --changelog or --changelog-to-stdout
if self.changelog_flag or bool(self.changelog_to_stdout):
raise NotAllowed(
"--changelog or --changelog-to-stdout is not allowed with --get-next"
)
# --get-next is a special case, taking precedence over config for 'update_changelog_on_bump'
self.changelog_config = False
# Setting dry_run to prevent any unwanted changes to the repo or files
self.dry_run = True
else:
# If user specified changelog_to_stdout, they probably want the
# changelog to be generated as well, this is the most intuitive solution
Copy link
Member

@Lee-W Lee-W Dec 6, 2024

Choose a reason for hiding this comment

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

I'm not so sure about that. If a user wants to print something, they may want to review it first before making any changes.

Copy link
Author

@Lowaiz Lowaiz Dec 9, 2024

Choose a reason for hiding this comment

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

This comment and the associated self.changelog_flag = (self.changelog_flag or bool(self.changelog_to_stdout) or self.changelog_config) is from the L210-L212 of the previous code. (with the inclusion of the new changelog_config change)

Copy link
Author

@Lowaiz Lowaiz Dec 9, 2024

Choose a reason for hiding this comment

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

If a user wants to print something, they may want to review it first before making any changes.

And, from what I understand from the the rest of the bump code, using changelog_to_stdout flag essentially put the changelog generation in dry run. So it will not change any files (my MR doesn't seem to change that behavior I guess).

self.changelog_flag = (
self.changelog_flag
or bool(self.changelog_to_stdout)
or self.changelog_config
)

current_tag_version: str = bump.normalize_tag(
current_version,
Expand Down Expand Up @@ -309,7 +315,7 @@ def __call__(self) -> None: # noqa: C901
)

files: list[str] = []
if self.changelog:
if self.changelog_flag:
args = {
"unreleased_version": new_tag_version,
"template": self.template,
Expand Down Expand Up @@ -356,7 +362,9 @@ def __call__(self) -> None: # noqa: C901
new_tag_version=new_tag_version,
message=message,
increment=increment,
changelog_file_name=changelog_cmd.file_name if self.changelog else None,
changelog_file_name=changelog_cmd.file_name
if self.changelog_flag
else None,
)

if is_files_only:
Expand All @@ -365,7 +373,7 @@ def __call__(self) -> None: # noqa: C901
# FIXME: check if any changes have been staged
git.add(*files)
c = git.commit(message, args=self._get_commit_args())
if self.retry and c.return_code != 0 and self.changelog:
if self.retry and c.return_code != 0 and self.changelog_flag:
# Maybe pre-commit reformatted some files? Retry once
logger.debug("1st git.commit error: %s", c.err)
logger.info("1st commit attempt failed; retrying once")
Expand Down Expand Up @@ -410,7 +418,9 @@ def __call__(self) -> None: # noqa: C901
current_tag_version=new_tag_version,
message=message,
increment=increment,
changelog_file_name=changelog_cmd.file_name if self.changelog else None,
changelog_file_name=changelog_cmd.file_name
if self.changelog_flag
else None,
)

# TODO: For v3 output this only as diagnostic and remove this if
Expand Down
20 changes: 20 additions & 0 deletions tests/commands/test_bump_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -1481,6 +1481,26 @@ def test_bump_get_next(mocker: MockFixture, capsys):
assert tag_exists is False


@pytest.mark.usefixtures("tmp_commitizen_project")
def test_bump_get_next_update_changelog_on_bump(
mocker: MockFixture, capsys, config_path
):
create_file_and_commit("feat: new file")
with open(config_path, "a", encoding="utf-8") as fp:
fp.write("update_changelog_on_bump = true\n")

testargs = ["cz", "bump", "--yes", "--get-next"]
mocker.patch.object(sys, "argv", testargs)
with pytest.raises(GetNextExit):
cli.main()

out, _ = capsys.readouterr()
assert "0.2.0" in out

tag_exists = git.tag_exist("0.2.0")
assert tag_exists is False


@pytest.mark.usefixtures("tmp_commitizen_project")
def test_bump_get_next__changelog_is_not_allowed(mocker: MockFixture):
create_file_and_commit("feat: new file")
Expand Down
Loading