Skip to content

[change] Included commit bodies in releaser changelog#702

Open
nemesifier wants to merge 2 commits into
masterfrom
releaser/include-long-desc
Open

[change] Included commit bodies in releaser changelog#702
nemesifier wants to merge 2 commits into
masterfrom
releaser/include-long-desc

Conversation

@nemesifier

@nemesifier nemesifier commented Jun 15, 2026

Copy link
Copy Markdown
Member

Checklist

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • I have updated the documentation.

Reference to Existing Issue

I have not opened an issue for this change.

Description of Changes

Includes commit bodies in releaser changelog entries so release notes do not lose important context when commit titles are terse.

Filters Git trailers, cherry-pick metadata, and Dependabot metadata blocks from generated changelog entries.

Converts markdown links to anonymous RST links for RST changelogs while preserving markdown links for markdown changelogs.

Screenshot

Not applicable.

Release changelogs need the full commit body when a title is too terse.

Git trailers and Dependabot metadata are filtered so generated entries
remain readable, while links use the target changelog format.
@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR extends the changelog generation pipeline to support multi-line commit messages. cliff.toml is updated so that each changelog section emits additional OW_CHANGELOG_BODY:<line> markers for every body line after the first. changelog.py introduces a marker constant, helpers to strip markers, convert Markdown links to RST, skip git trailers and Dependabot metadata blocks, and format body lines with correct indentation and blank lines. The process_changelog function gains a changelog_format parameter and runs the full new pipeline before reordering the Dependencies section, which is rewritten to group multi-line bumped entries. release.py passes the format explicitly. The sample fixture and three new tests cover body inclusion, trailer/Dependabot exclusion, and Markdown link preservation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Bug Fixes ❌ Error The PR contains a bug fix request in review comments that has not been implemented. The Dependabot metadata detection (lines 111-119 of changelog.py) lacks the recommended constraints and is missin... Apply the suggested fix from review comments: add is_body_line and check and constrain lookahead to lines[index + 1:] with OW_CHANGELOG_BODY: check. Add regression test with commit containing "---" in body before a Dependabot commit.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title '[change] Included commit bodies in releaser changelog' accurately describes the main change and follows the required format with proper type prefix.
Description check ✅ Passed The PR description addresses the main points but does not reference an existing issue as required by the template, and documentation updates are noted as pending.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch releaser/include-long-desc

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@openwisp-companion

Copy link
Copy Markdown

The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (1/3).

@openwisp-companion

Copy link
Copy Markdown

The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (2/3).

@nemesifier nemesifier self-assigned this Jun 15, 2026
@openwisp-companion

Copy link
Copy Markdown

The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (3/3).

@openwisp-companion

Copy link
Copy Markdown

Test Failures in Selenium Tests

Hello @nemesifier,
(Analysis for commit 03ac106)

The CI failed due to AssertionError: None != [] in the test_get_browser_logs test across multiple jobs. This indicates that the get_browser_logs function is returning None instead of an empty list when no browser logs are expected.

Fix:

Modify the test_get_browser_logs test to handle the case where get_browser_logs() might return None. A simple way to do this is to assert that the result is either an empty list or None, or to ensure get_browser_logs() always returns a list.

For example, you could change the assertion from:

self.assertEqual(self.get_browser_logs(), [])

to:

self.assertIsNone(self.get_browser_logs()) # If None is the expected outcome

or, if an empty list is always expected:

self.assertEqual(self.get_browser_logs() or [], [])

Alternatively, ensure the get_browser_logs function itself consistently returns an empty list when no logs are found, rather than None.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@openwisp_utils/releaser/changelog.py`:
- Around line 111-119: The Dependabot metadata detection in the conditional
block starting at dependabot_metadata_start.match currently scans all remaining
lines in the file for updated-dependencies:, causing it to incorrectly identify
metadata from later commits and skip unrelated content from the current block.
Modify the lookahead check to only scan contiguous lines that belong to the same
OW_CHANGELOG_BODY block (stopping when reaching a line that marks the end of the
current block), rather than checking all remaining lines. Additionally, add a
regression test case that verifies a normal changelog body containing ---
followed by another commit with Dependabot metadata does not cause unrelated
lines from the first body to be dropped.
🪄 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: 81632463-8629-4e28-929f-0e97f4d77e43

📥 Commits

Reviewing files that changed from the base of the PR and between 7af7b28 and e4f7ad1.

📒 Files selected for processing (5)
  • openwisp_utils/cliff.toml
  • openwisp_utils/releaser/changelog.py
  • openwisp_utils/releaser/release.py
  • openwisp_utils/releaser/tests/samples/changelogs/full_changelog.rst
  • openwisp_utils/releaser/tests/test_changelog.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/release.py
  • openwisp_utils/releaser/changelog.py
  • openwisp_utils/releaser/tests/test_changelog.py
**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Place imports at the top of the file; only defer imports when necessary (e.g., Django model imports inside functions or methods where the app registry is not yet ready)
Avoid unnecessary blank lines inside function and method bodies
Add or update tests for every behavior change
Run openwisp-qa-format after editing
Prefer in-process tests so coverage tools can measure changed code
When checking coverage for a changed module, use python -m pytest <test_path> --cov=<dotted.module.path> --cov-report=term-missing
Watch for unsafe file paths, unsafe subprocess usage, token or secret exposure, and changes that could weaken QA or release safeguards
Write comments and docstrings only when they explain why code is shaped a certain way; place comments before the relevant code block instead of scattering them inside it

Files:

  • openwisp_utils/releaser/release.py
  • openwisp_utils/releaser/changelog.py
  • openwisp_utils/releaser/tests/test_changelog.py
**/*.{md,rst,txt}

📄 CodeRabbit inference engine (AGENTS.md)

Update documentation when behavior, settings, public APIs, setup steps, QA rules, or supported versions change

Files:

  • openwisp_utils/releaser/tests/samples/changelogs/full_changelog.rst
**/tests/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

For bug fixes, write the regression test first, run it against the unfixed code, confirm it fails for the expected reason, then implement the fix

Files:

  • openwisp_utils/releaser/tests/test_changelog.py
🧠 Learnings (1)
📚 Learning: 2026-06-15T22:45:01.529Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 701
File: setup.py:72-72
Timestamp: 2026-06-15T22:45:01.529Z
Learning: When reviewing changes that require a minimum Selenium Python version for specific APIs (e.g., BiDi features like `Options.enable_bidi`, console/message handlers like `Script.add_console_message_handler`, or event handlers like `BrowsingContext.add_event_handler`), do not use the Selenium API docs site (`https://www.selenium.dev/selenium/docs/api/py/`) as the source of introduction/version history, since it reflects the latest API shape. Instead, confirm feature availability by checking the official Python changelog (`py/CHANGES` in the Selenium repo) and/or inspecting the Selenium source (tags/commits corresponding to candidate versions). Only accept the stated minimum version after verifying that the referenced APIs exist in that Selenium version (e.g., Selenium 4.32.0 includes the BiDi APIs used by openwisp-utils’ BiDi-related PR `#701` as of that version).

Applied to files:

  • openwisp_utils/releaser/release.py
  • openwisp_utils/releaser/changelog.py
  • openwisp_utils/releaser/tests/test_changelog.py
🪛 ast-grep (0.43.0)
openwisp_utils/releaser/tests/test_changelog.py

[error] 137-137: Command coming from incoming request
Context: subprocess.run(["git", "add", "."], check=True, capture_output=True)
Note: [CWE-20].

(subprocess-from-request)


[error] 138-143: Command coming from incoming request
Context: subprocess.run(
["git", "commit", "--file=-"],
input=message.encode("utf-8"),
check=True,
capture_output=True,
)
Note: [CWE-20].

(subprocess-from-request)

🔇 Additional comments (3)
openwisp_utils/cliff.toml (1)

20-24: LGTM!

Also applies to: 44-48, 59-63, 74-78, 90-94

openwisp_utils/releaser/release.py (1)

222-224: LGTM!

openwisp_utils/releaser/tests/samples/changelogs/full_changelog.rst (1)

12-14: LGTM!

Also applies to: 18-22, 26-27, 37-40, 49-50, 56-61, 64-69, 72-77, 80-88, 96-97, 101-107

Comment on lines +111 to +119
if dependabot_metadata_start.match(stripped_line):
has_dependabot_metadata = any(
_get_changelog_line_content(remaining_line) == "updated-dependencies:"
for remaining_index, remaining_line in enumerate(lines)
if remaining_index > index
)
if has_dependabot_metadata:
skip_dependabot_metadata = True
continue

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Dependabot metadata detection can drop unrelated commit body content.

When a body line is ---, the current lookahead scans all remaining lines for updated-dependencies:. If a later commit contains Dependabot metadata, skip mode starts too early and removes unrelated lines until ....

Please constrain the check to subsequent contiguous OW_CHANGELOG_BODY: lines in the same block, and add a regression case where a normal body contains --- before another commit that has Dependabot metadata.

Suggested fix
-        if dependabot_metadata_start.match(stripped_line):
-            has_dependabot_metadata = any(
-                _get_changelog_line_content(remaining_line) == "updated-dependencies:"
-                for remaining_index, remaining_line in enumerate(lines)
-                if remaining_index > index
-            )
+        if is_body_line and dependabot_metadata_start.match(stripped_line):
+            has_dependabot_metadata = False
+            for remaining_line in lines[index + 1 :]:
+                if not remaining_line.strip().startswith(CHANGELOG_BODY_MARKER):
+                    break
+                remaining_content = _get_changelog_line_content(remaining_line)
+                if remaining_content == "updated-dependencies:":
+                    has_dependabot_metadata = True
+                    break
+                if remaining_content:
+                    break
             if has_dependabot_metadata:
                 skip_dependabot_metadata = True
                 continue
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@openwisp_utils/releaser/changelog.py` around lines 111 - 119, The Dependabot
metadata detection in the conditional block starting at
dependabot_metadata_start.match currently scans all remaining lines in the file
for updated-dependencies:, causing it to incorrectly identify metadata from
later commits and skip unrelated content from the current block. Modify the
lookahead check to only scan contiguous lines that belong to the same
OW_CHANGELOG_BODY block (stopping when reaching a line that marks the end of the
current block), rather than checking all remaining lines. Additionally, add a
regression test case that verifies a normal changelog body containing ---
followed by another commit with Dependabot metadata does not cause unrelated
lines from the first body to be dropped.

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.

1 participant