Copilot commented on code in PR #4425:
URL: https://github.com/apache/solr/pull/4425#discussion_r3232632300
##########
dev-tools/scripts/releaseWizard.yaml:
##########
@@ -771,7 +771,7 @@ groups:
confirm_each_command: false
commands:
- !Command
- cmd: python3 -u dev-tools/scripts/logchange.py prepare --version {{
release_version }} --release-branch {{ release_branch }} --gradle-cmd {{
gradle_cmd }} --commit
+ cmd: python3 -u dev-tools/scripts/logchange.py prepare --version {{
release_version }} --release-branch {{ release_branch }} --gradle-cmd {{
gradle_cmd }} --rc-number {{ rc_number }} --commit
comment: Move unreleased entries to version folder, regenerate
CHANGELOG.md, and commit
Review Comment:
This command now runs logchange.py prepare with --commit, which (per the
updated logchange.py) creates two commits (YAML first, generated files second).
The todo comment still implies a single commit; please update the wording to
avoid confusing release managers when they see two commits on the release
branch.
This issue also appears on line 1246 of the same file.
##########
dev-tools/scripts/logchange.py:
##########
@@ -248,26 +251,41 @@ def cmd_prepare(args, git_root):
shutil.copy2(src, dst)
src.unlink()
- print(f"\n[3] Running logchangeGenerate and stripping [unreleased] block")
- run_logchange_generate(gradle_cmd, git_root, dry_run=dry_run)
-
if do_commit:
- print(f"\n[4] Staging changelog/ and CHANGELOG.md")
+ # Commit A: changelog/ YAML files only (version-summary.md is stale
until generate runs)
+ print(f"\n[3] Staging changelog/ entries for {version_label} (commit
A)")
ensure_unreleased_gitkeep(git_root, dry_run=dry_run)
- git(["add", "changelog", "CHANGELOG.md"], cwd=git_root,
dry_run=dry_run)
+ git(["add", "changelog"], cwd=git_root, dry_run=dry_run)
+ # Unstage version-summary.md — it is stale until logchangeGenerate runs
+ git(["restore", "--staged", version_summary], cwd=git_root,
dry_run=dry_run, check=False)
if not dry_run and not has_staged_changes(git_root):
- print("\nNothing staged — changelog is already up to date.")
- return
+ print("\nNothing staged — changelog entries are already up to
date.")
+ else:
+ msg_a = f"Changelog entries for {version_label}"
+ print(f"\n[4] Committing: {msg_a!r}")
+ git(["commit", "-m", msg_a], cwd=git_root, dry_run=dry_run)
Review Comment:
The staged-change detection here uses has_staged_changes(git_root), which
checks for *any* staged content. If the user has unrelated files already
staged, this block may commit those unrelated changes (even when changelog
entries are already up to date), producing a misleading commit message.
Consider enforcing a clean index/worktree before running with --commit, or
change the logic to verify staged changes are limited to the intended paths
(e.g., changelog/ excluding version-summary.md) before deciding to commit.
This issue also appears on line 341 of the same file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
