[fixes] Automate branch selection for porting changelog #646#647
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
🧰 Additional context used📓 Path-based instructions (1)**/*.{py,html,txt}📄 CodeRabbit inference engine (Custom checks)
Files:
🧠 Learnings (5)📚 Learning: 2026-03-14T20:44:14.568ZApplied to files:
📚 Learning: 2026-03-14T20:44:14.568ZApplied to files:
📚 Learning: 2026-03-14T20:44:14.568ZApplied to files:
📚 Learning: 2026-03-14T20:44:14.568ZApplied to files:
📚 Learning: 2026-03-14T20:44:14.568ZApplied to files:
🔇 Additional comments (5)
📝 WalkthroughWalkthroughAdded a helper function Sequence Diagram(s)sequenceDiagram
participant User
participant Releaser
participant LocalGit
participant GitHub
User->>Releaser: run port_changelog_to_main
Releaser->>LocalGit: branch_exists("main")?
LocalGit-->>Releaser: exists_main (true/false)
Releaser->>LocalGit: branch_exists("master")?
LocalGit-->>Releaser: exists_master (true/false)
alt neither exists
Releaser->>User: print notice and exit
else both exist
Releaser->>User: prompt select target branch
User-->>Releaser: selected_branch or cancel
alt user selected
Releaser->>Releaser: update changelog for selected_branch
Releaser->>GitHub: create PR target=selected_branch
else user canceled
Releaser->>User: exit without creating PR
end
else only main exists
Releaser->>Releaser: update changelog for "main"
Releaser->>GitHub: create PR target="main"
else only master exists
Releaser->>Releaser: update changelog for "master"
Releaser->>GitHub: create PR target="master"
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge OverviewThe automated branch detection feature from Issue #646 has been reverted. The PR now contains bot improvements, changelog bot workflows, and Selenium test fixes. Changes Since Last Review
Files Reviewed (18 files changed)Bot Improvements:
Changelog Bot Workflows:
Selenium Fixes:
Documentation:
TestingAll functionality is well-tested. The reverted branch detection tests have been removed appropriately. Reviewed by kimi-k2.5-0127 · 437,665 tokens |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openwisp_utils/releaser/release.py`:
- Around line 187-190: The current branch-selection block sets main_branch =
"master" even when both main_exists and master_exists are False; update the
logic in the same function/block that uses the main_exists and master_exists
variables (the branch-selection around main_branch) to explicitly check
master_exists and, if neither main_exists nor master_exists is True, raise a
clear RuntimeError (or custom exception) explaining that neither 'main' nor
'master' exist locally instead of defaulting to "master"; ensure the error
message references 'main' and 'master' so callers get an early, descriptive
failure.
In `@openwisp_utils/releaser/tests/test_release.py`:
- Around line 185-236: The tests calling port_changelog_to_main are missing a
patch for the external formatter and can invoke real formatting logic; patch
openwisp_utils.releaser.release.format_file_with_docstrfmt in each of the three
tests (test_port_changelog_only_master_exists,
test_port_changelog_only_main_exists,
test_port_changelog_both_branches_prompts_user) and set it to a no-op (e.g., a
MagicMock or simple stub) so the formatter is not executed during the test run;
keep existing patches (branch_exists, subprocess.run, update_changelog_file,
questionary) and ensure the patched format_file_with_docstrfmt is added to the
decorator list and its mock is passed into the test functions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b4998e12-ceb2-4dc6-abd5-dea1fa8a11b0
📒 Files selected for processing (3)
openwisp_utils/releaser/release.pyopenwisp_utils/releaser/tests/test_release.pyopenwisp_utils/releaser/utils.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Kilo Code Review
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{py,html,txt}
📄 CodeRabbit inference engine (Custom checks)
For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework
Files:
openwisp_utils/releaser/utils.pyopenwisp_utils/releaser/release.pyopenwisp_utils/releaser/tests/test_release.py
🧠 Learnings (1)
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Changes: Update tests to cover non-trivial changes and ensure proper validation of modified behavior
Applied to files:
openwisp_utils/releaser/tests/test_release.py
🔇 Additional comments (2)
openwisp_utils/releaser/utils.py (1)
88-97: Branch existence helper is correct for local-branch detection.Line 90-96 uses
git show-ref --verify --quietand a return-code check, which cleanly implements the intended local branch presence probe.openwisp_utils/releaser/tests/test_release.py (1)
158-183: Good coverage for the “both branches exist” prompt/cancel path.This segment cleanly exercises the interactive branch-selection flow and cancellation behavior.
Flake8 and Commit Message FailuresHello @prathmeshkulkarni-coder,
For example: |
8c6dbb6 to
c58dd22
Compare
QA Failures: Black and Flake8Hello @prathmeshkulkarni-coder, The CI failed due to code style issues detected by Black and Flake8. Fix: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openwisp_utils/releaser/release.py`:
- Around line 192-194: Replace the grammatically incorrect print message in
release.py that currently prints "Could not find neither 'master' nor 'main'
branch locally. Skipping changelog porting." (the print(...) call shown in the
diff) with a corrected sentence such as "Could not find either 'master' or
'main' branch locally. Skipping changelog porting." so the CLI output is
grammatically correct and clear.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6f2a3979-8436-4cb3-b9c0-0be670c3838b
📒 Files selected for processing (3)
openwisp_utils/releaser/release.pyopenwisp_utils/releaser/tests/test_release.pyopenwisp_utils/releaser/utils.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Kilo Code Review
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{py,html,txt}
📄 CodeRabbit inference engine (Custom checks)
For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework
Files:
openwisp_utils/releaser/utils.pyopenwisp_utils/releaser/release.pyopenwisp_utils/releaser/tests/test_release.py
🧠 Learnings (4)
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Changes: Update tests to cover non-trivial changes and ensure proper validation of modified behavior
Applied to files:
openwisp_utils/releaser/tests/test_release.py
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Bug Fixes: Ensure the test is deterministic and not flaky - flag tests that depend on timing, sleeps, specific timezones, system time, randomness without fixed seed, race conditions, concurrency timing, network access, external services, filesystem state, environment-specific configuration, execution order, shared global state, hardcoded ports, or unawaited async operations
Applied to files:
openwisp_utils/releaser/tests/test_release.py
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Bug Fixes: Ensure the regression test fails if the patch is removed or commented out
Applied to files:
openwisp_utils/releaser/tests/test_release.py
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Features: Add tests for new features and ensure coverage does not decrease significantly; prefer Selenium browser tests for UI-impacting features
Applied to files:
openwisp_utils/releaser/tests/test_release.py
🔇 Additional comments (5)
openwisp_utils/releaser/utils.py (1)
88-97: Branch existence helper looks correct and safe.The implementation is side-effect free and correctly maps Git return codes to a boolean existence check.
openwisp_utils/releaser/release.py (1)
180-191: Branch-target selection now correctly implements the intended behavior.Prompting only when both branches exist and auto-selecting otherwise is the right control flow.
Also applies to: 196-196
openwisp_utils/releaser/tests/test_release.py (3)
158-183: Good test update for the “both branches exist” path.Mocking
branch_existshere makes the branch-selection path explicit and keeps this test aligned with the new release logic.
185-262: Strong coverage for all branch-presence combinations.These tests validate auto-selection (
main/master), interactive selection when both exist, and early exit when neither exists.
328-356: Nice hardening of markdown/skip-PR tests with branch checks mocked.Patching
branch_existsin these cases removes ambiguity and keeps behavior deterministic across environments.
c58dd22 to
63af5aa
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
63af5aa to
e338014
Compare
3bb33ca to
61d10ef
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
openwisp_utils/releaser/release.py (1)
193-194:⚠️ Potential issue | 🟡 MinorFix grammar in the “missing branches” message.
At Line 193, the sentence is grammatically incorrect (“branch were”). Please switch to plural or use “either/or”.
Suggested wording
- "Neither 'master' nor 'main' branch were found locally. " + "Neither 'master' nor 'main' branches were found locally. " "Skipping changelog porting."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openwisp_utils/releaser/release.py` around lines 193 - 194, The literal error message string "Neither 'master' nor 'main' branch were found locally. Skipping changelog porting." is grammatically incorrect; update it in openwisp_utils/releaser/release.py (the message used where missing branches are reported) to a correct form such as "Neither 'master' nor 'main' branches were found locally. Skipping changelog porting." or alternately "Neither 'master' nor 'main' branch was found locally. Skipping changelog porting." to fix subject-verb agreement.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@openwisp_utils/releaser/release.py`:
- Around line 193-194: The literal error message string "Neither 'master' nor
'main' branch were found locally. Skipping changelog porting." is grammatically
incorrect; update it in openwisp_utils/releaser/release.py (the message used
where missing branches are reported) to a correct form such as "Neither 'master'
nor 'main' branches were found locally. Skipping changelog porting." or
alternately "Neither 'master' nor 'main' branch was found locally. Skipping
changelog porting." to fix subject-verb agreement.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fdca9abb-b07c-455f-b9e6-ac4d19367ee2
📒 Files selected for processing (4)
openwisp_utils/releaser/release.pyopenwisp_utils/releaser/tests/test_release.pyopenwisp_utils/releaser/tests/test_utils.pyopenwisp_utils/releaser/utils.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Kilo Code Review
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{py,html,txt}
📄 CodeRabbit inference engine (Custom checks)
For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework
Files:
openwisp_utils/releaser/tests/test_utils.pyopenwisp_utils/releaser/utils.pyopenwisp_utils/releaser/release.pyopenwisp_utils/releaser/tests/test_release.py
🧠 Learnings (5)
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Changes: Update tests to cover non-trivial changes and ensure proper validation of modified behavior
Applied to files:
openwisp_utils/releaser/tests/test_release.py
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Bug Fixes: Ensure the test is deterministic and not flaky - flag tests that depend on timing, sleeps, specific timezones, system time, randomness without fixed seed, race conditions, concurrency timing, network access, external services, filesystem state, environment-specific configuration, execution order, shared global state, hardcoded ports, or unawaited async operations
Applied to files:
openwisp_utils/releaser/tests/test_release.py
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Bug Fixes: Ensure the regression test fails if the patch is removed or commented out
Applied to files:
openwisp_utils/releaser/tests/test_release.py
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Features: Add tests for new features and ensure coverage does not decrease significantly; prefer Selenium browser tests for UI-impacting features
Applied to files:
openwisp_utils/releaser/tests/test_release.py
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Bug Fixes: Ensure a regression test is included that reproduces the bug scenario
Applied to files:
openwisp_utils/releaser/tests/test_release.py
🔇 Additional comments (5)
openwisp_utils/releaser/utils.py (1)
88-96:branch_existsimplementation is solid.Clear and correct local-branch detection using git refs; return-code handling is straightforward.
openwisp_utils/releaser/tests/test_utils.py (1)
312-326: Good coverage forbranch_existsbehavior.The new tests cover both return-code branches and verify lookup against
refs/heads/<branch>.openwisp_utils/releaser/release.py (1)
180-190: Branch auto-selection logic matches the requested behavior.This correctly avoids prompting when only one of
master/mainexists and keeps prompting when both exist.openwisp_utils/releaser/tests/test_release.py (2)
185-286: Excellent scenario coverage for branch-selection behavior.These tests cover the full matrix (only
master, onlymain, both, neither, cancelled) and lock in the regression surface for issue#646.
158-166: Good update to patchbranch_existsin existing flow tests.This keeps the tests deterministic while aligning older test paths with the new branch-detection behavior.
Based on learnings: "Changes: Update tests to cover non-trivial changes and ensure proper validation of modified behavior" and "Bug Fixes: Ensure the test is deterministic and not flaky - flag tests that depend on timing, sleeps, specific timezones, system time, randomness without fixed seed, race conditions, concurrency timing, network access, external services, filesystem state, environment-specific configuration, execution order, shared global state, hardcoded ports, or unawaited async operations".Also applies to: 352-377
Implemented automated branch detection to avoid choosing between master and main when only one exists. Fixes openwisp#646
61d10ef to
1a5cf66
Compare
|
@nemesifier ,The 0.07% coverage drop in selenium.py is unrelated to this PR I haven't modified that file. My changes in the releaser/ module maintained and increased coverage. |
Implemented automated branch detection to avoid choosing between master and main when only one exists.
Fixes #646
Checklist
Reference to Existing Issue
Closes #646
Description of Changes
This PR automates the branch selection logic when porting changelogs during a bugfix release. Previously, the tool would always prompt the user to choose between master and main, even if only one of them existed.
Screenshot
Please include any relevant screenshots.