New process for (un)released changelog/migration guide #8698
Conversation
…ead of from an unreleased changelog/migration guide
|
Note Reviews pausedUse the following commands to manage reviews:
📝 WalkthroughWalkthroughA new script for generating changelog entry files per pull request has been introduced, and the changelog/migration guide update script has been refactored into modular functions. The process for adding changelog entries now uses individual files named after PR numbers, reducing merge conflicts and streamlining release documentation. Unreleased changelog and migration files were replaced by warning messages directing contributors to the new workflow. Changes
Possibly related PRs
Suggested labels
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
philippotto
changed the title
…the correct pr number and creates a template file that the developer can edit
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
tools/create-changelog-entry.py (2)
57-64: Handle already-existing files more gracefullyInstead of an unconditional hard exit, consider asking the user whether to overwrite or pick a different PR number. This avoids aborting scripted flows.
60-66: Preferpathlib.Pathfor cross-platform path handling
Path("unreleased_changes") / f"{pr_number}.md"improves readability and avoids manualos.path.join.unreleased_changes/8698.md (1)
1-2: Tighten wording & avoid repetitionThe bullet already starts with “Changed …”, directly under “### Changed”. Consider dropping the verb or rephrasing to avoid feeling repetitive.
tools/make-changelog.py (2)
108-110: Rename single-letter loop variables
lis flagged by Ruff (E741) as ambiguous. Rename tolinefor clarity.- lines = [l.rstrip() for l in f] + lines = [line.rstrip() for line in f]Apply similarly at lines 128 and 148.
Also applies to: 127-129, 148-149
34-46: Gracefully handle missingunreleased_changesdirectory
os.listdirraises if the folder is absent. Add:if not os.path.isdir(UNRELEASED_CHANGES_DIR): print("No unreleased change files found, nothing to do.") return changelog_sections, migration_sectionsto avoid a hard crash in clean repos.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tools/create-changelog-entry.py(1 hunks)tools/make-changelog.py(1 hunks)unreleased_changes/8698.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
unreleased_changes/8698.md
[duplication] ~1-~1: Possible typo: you repeated a word.
Context: ### Changed - Changed the process for adding changelog and mi...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~2-~2: Possible missing comma found.
Context: ...uest. The file will contain the correct format with dummy values for the changelog and...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Pylint (3.3.7)
tools/create-changelog-entry.py
[refactor] 55-55: Consider using 'sys.exit' instead
(R1722)
[refactor] 63-63: Consider using 'sys.exit' instead
(R1722)
tools/make-changelog.py
[refactor] 105-105: Too many arguments (6/5)
(R0913)
[refactor] 105-105: Too many positional arguments (6/5)
(R0917)
🪛 Ruff (0.11.9)
tools/make-changelog.py
109-109: Ambiguous variable name: l
(E741)
128-128: Ambiguous variable name: l
(E741)
148-148: Ambiguous variable name: l
(E741)
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
tools/make-changelog.py (2)
110-112: Rename loop variable fromltolineSingle-letter
lis flagged by Ruff (E741) for being visually ambiguous. Renaming improves readability and silences the linter.- lines = [l.rstrip() for l in f] + lines = [line.rstrip() for line in f]
91-104: Always emit section headers in migration guide for consistencyWhen the “Migration” list is empty the header is skipped, leading to inconsistent structure compared to “Postgres Evolutions” (which always prints “None.”). Consider printing an explicit “None.” for Migration too.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tools/create-changelog-entry.py(1 hunks)tools/make-changelog.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tools/create-changelog-entry.py
🧰 Additional context used
🪛 Ruff (0.11.9)
tools/make-changelog.py
111-111: Ambiguous variable name: l
(E741)
130-130: Ambiguous variable name: l
(E741)
150-150: Ambiguous variable name: l
(E741)
174-174: Do not use bare except
(E722)
🪛 Pylint (3.3.7)
tools/make-changelog.py
[refactor] 107-107: Too many arguments (6/5)
(R0913)
[refactor] 107-107: Too many positional arguments (6/5)
(R0917)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: backend-tests
- GitHub Check: build-smoketest-push
- GitHub Check: frontend-tests
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
unreleased_changes/8638.md (1)
1-3: Critical: File name does not match PR number.This entry file is named
8638.mdbut belongs to PR #8698. Automated tooling relies on the filename matching the PR number. Please rename it to8698.mdso your changelog entry is correctly picked up.
🧹 Nitpick comments (7)
unreleased_changes/8638.md (1)
2-2: Use American English and imperative mood.Please update the bullet to:
“Clear caches when deleting a dataset to ensure that if a dataset with the same name is uploaded afterward, only new data is loaded.”
This fixes the ‘afterwards’→‘afterward’ spelling and switches to the project’s preferred imperative style.unreleased_changes/8687.md (1)
2-2: Refine bullet wording and include PR reference
Avoid repeating “Fixed” in the bullet text. Also append the PR number for traceability:- - Fixed that activating a mapping got stuck when a dataset was opened in "view" mode. + - Activating a mapping got stuck when a dataset was opened in "view" mode. (#8687)unreleased_changes/8662.md (1)
1-2: Avoid repeating “Added”.You have
### Addedfollowed by a bullet starting with “Added…”. Consider dropping the verb from the bullet to reduce duplication. For example:- Ability to duplicate trees in skeleton annotations (all nodes, edges, and properties) via the skeleton tab context menu.unreleased_changes/8688.md (1)
1-2: Use imperative mood in changelog entries
Changelog bullet points should be written in present-tense imperative form. For example:-### Fixed -- Fixed the contrast of the WelcomeToast buttons. Updated `antd` to version `5.22`. +### Fixed + +- Fix contrast of `WelcomeToast` buttons and update `antd` to version 5.22.unreleased_changes/8695.md (1)
1-2: Remove redundant “Fixed” in the bullet.
Under the### Fixedheader, the bullet already implies a fix—start with the issue description for clarity.-### Fixed -- Fixed a regression that led to incorrect behavior when trying to jump to the last branchpoint even though no branchpoint existed. +### Fixed + +- A regression that caused incorrect behavior when attempting to jump to the last branchpoint when none existed.unreleased_changes/8676.md (2)
2-2: Use imperative mood and remove redundant “Fixed” in the bullet entry.Under the
### Fixedheader, avoid repeating “Fixed”. For example:- - Fixed a race condition when starting proofreading with a split action. + - Fix a race condition when starting proofreading with a split action.
1-3: Include a “Migration guide” section for consistency.The template normally includes a
### Migration guidesection. Even if no migration steps are required, please add the header (e.g. with “None”) to keep all entry files uniform.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
CHANGELOG.unreleased.md(1 hunks)MIGRATIONS.unreleased.md(1 hunks)unreleased_changes/8492.md(1 hunks)unreleased_changes/8518.md(1 hunks)unreleased_changes/8542.md(1 hunks)unreleased_changes/8598.md(1 hunks)unreleased_changes/8622.md(1 hunks)unreleased_changes/8638.md(1 hunks)unreleased_changes/8662.md(1 hunks)unreleased_changes/8676.md(1 hunks)unreleased_changes/8686.md(1 hunks)unreleased_changes/8687.md(1 hunks)unreleased_changes/8688.md(1 hunks)unreleased_changes/8695.md(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- unreleased_changes/8622.md
- unreleased_changes/8518.md
- unreleased_changes/8598.md
- unreleased_changes/8492.md
- unreleased_changes/8686.md
- unreleased_changes/8542.md
- CHANGELOG.unreleased.md
- MIGRATIONS.unreleased.md
🧰 Additional context used
🪛 LanguageTool
unreleased_changes/8638.md
[locale-violation] ~2-~2: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...ew dataset by the same name is uploaded afterwards, only new data is loaded.
(AFTERWARDS_US)
unreleased_changes/8662.md
[duplication] ~1-~1: Possible typo: you repeated a word.
Context: ### Added - Added the ability to duplicate trees in skele...
(ENGLISH_WORD_REPEAT_RULE)
unreleased_changes/8676.md
[duplication] ~1-~1: Possible typo: you repeated a word.
Context: ### Fixed - Fixed a race condition when starting proofrea...
(ENGLISH_WORD_REPEAT_RULE)
unreleased_changes/8687.md
[duplication] ~1-~1: Possible typo: you repeated a word.
Context: ### Fixed - Fixed that activating a mapping got stuck whe...
(ENGLISH_WORD_REPEAT_RULE)
unreleased_changes/8688.md
[duplication] ~1-~1: Possible typo: you repeated a word.
Context: ### Fixed - Fixed the contrast of the WelcomeToast button...
(ENGLISH_WORD_REPEAT_RULE)
unreleased_changes/8695.md
[duplication] ~1-~1: Possible typo: you repeated a word.
Context: ### Fixed - Fixed a regression that led to incorrect beha...
(ENGLISH_WORD_REPEAT_RULE)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build-smoketest-push
- GitHub Check: frontend-tests
- GitHub Check: backend-tests
|
@coderabbitai ignore |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
tools/make-changelog.py (4)
7-7: Fix the VERSION_REGEX pattern to properly validate version strings.The regex pattern has two issues that were previously flagged:
- The dots aren't escaped, so the pattern matches unintended characters
- The pattern allows trailing characters and rejects valid versions without patch numbers
Apply this fix:
-VERSION_REGEX = r"[\d]{1,2}\.[\d]{1,2}\.[\d]{0,2}" +VERSION_REGEX = r"^\d{1,2}\.\d{1,2}(?:\.\d{1,2})?$"And update the validation call:
- if not re.match(VERSION_REGEX, this_version): + if not re.fullmatch(VERSION_REGEX, this_version):
26-33: Handle the potential StopIteration exception.The
next(matches)call will raiseStopIterationif no version header is found in the file, which can happen with a first release.- first_match = next(matches) + try: + first_match = next(matches) + except StopIteration: + raise ValueError( + "Could not find a previous version header (## [<version>]) " + f"in the changelog/migration file" + )
49-58: Add directory existence check to prevent FileNotFoundError.The script will crash if the
unreleased_changesdirectory doesn't exist, which could happen in a fresh repository checkout.+ if not os.path.isdir(UNRELEASED_CHANGES_DIR): + print(f"Warning: '{UNRELEASED_CHANGES_DIR}' directory not found – skipping aggregation.") + return changelog_sections, migration_sections + for file_name in sorted(os.listdir(UNRELEASED_CHANGES_DIR)):
177-178: Replace bare except with specific exception handling.The bare except clause catches all exceptions, including system exits and keyboard interrupts, which can mask important errors.
- except: - print(f"Warning: Could not git rm {path}. Was it added to git?") + except (subprocess.CalledProcessError, FileNotFoundError) as exc: + print(f"Warning: Could not 'git rm' {path}: {exc}. Was it added to git?")
🧹 Nitpick comments (3)
tools/make-changelog.py (3)
36-91: Refactor the parse_change_files function to reduce complexity.The function has too many nested blocks (6/5) and branches (13/12) according to static analysis. Consider extracting helper functions for parsing individual files and processing sections.
Consider this refactor:
+def parse_single_change_file(file_path, pr_number): + """Parse a single changelog entry file and return sections.""" + with open(file_path) as f: + content = f.read() + + fragments = re.split(r"\n### (.+?)\n", "\n" + content, flags=re.MULTILINE) + return dict(zip(fragments[1::2], fragments[2::2])) + +def process_changelog_section(section, body, pr_number, file_name): + """Process a changelog section and return entries with PR links.""" + entries = [] + for line in body.strip().splitlines(): + if line.strip().startswith("- "): + pr_link = f"[#{pr_number}](https://github.com/{GITHUB_REPO}/pull/{pr_number})" + if pr_link in line: + print(f"Warning: {file_name} already contains a link to its PR.") + entries.append(f"{line.strip()} {pr_link}") + else: + print(f"Warning: {file_name} contains lines that don't begin with a dash.") + return entries
114-114: Use descriptive variable names instead of ambiguous single letters.The variable
lis ambiguous and should be renamed tolinefor better readability.- lines = [l.rstrip() for l in f] + lines = [line.rstrip() for line in f]Apply this change to all three occurrences in the file.
Also applies to: 133-133, 153-153
110-129: Consider reducing the number of function parameters.The function has 6 parameters which exceeds the recommended limit. Consider grouping related parameters into a data structure.
+from dataclasses import dataclass + +@dataclass +class ReleaseInfo: + version: str + last_version: str + last_idx: int + today_str: str + -def update_released_file( - file_path, version, last_version, last_idx, today_str, new_lines -): +def update_released_file(file_path, release_info: ReleaseInfo, new_lines):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tools/make-changelog.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
tools/make-changelog.py
114-114: Ambiguous variable name: l
(E741)
133-133: Ambiguous variable name: l
(E741)
153-153: Ambiguous variable name: l
(E741)
177-177: Do not use bare except
(E722)
🪛 Pylint (3.3.7)
tools/make-changelog.py
[refactor] 49-89: Too many nested blocks (6/5)
(R1702)
[refactor] 36-36: Too many branches (13/12)
(R0912)
[refactor] 110-110: Too many arguments (6/5)
(R0913)
[refactor] 110-110: Too many positional arguments (6/5)
(R0917)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-smoketest-push
- GitHub Check: frontend-tests
🔇 Additional comments (1)
tools/make-changelog.py (1)
182-200: The main execution flow looks well-structured.The refactored main execution provides clear separation of concerns and good error messaging. The modular approach makes the script more maintainable than the previous version.
✅ Actions performedReviews paused. |
Inspired by towncrier I adapted the changelog/migration guide. Quoting from the changelog
Why I decided against towncrier:
Steps to test:
python3 tools/create-changelog-entry.py(will infer the pr number ifghis available and a pr exists for the current branch; otherwise it will ask for a number)python3 tools/make-changelog.py 25.06.3TODOs:
after the first review:
Issues:
(Please delete unneeded items, merge only when none are left open)