When prepping a release, combine the dependency bump changelog entries. #2181
Conversation
…ats might break other things.
…Update prep-release to use it. Also apply a couple fixes that are alread in the release branch (and will be in main shortly). Also tweak the step 4 and 5 names to provide more context, and fix the verbose output header when recombining the sections.
WalkthroughThe pull request introduces several updates primarily focused on changelog management and dependency tracking within scripts. Key changes include the renaming of the release notes file to Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
…uld actually be RELEASE_CHANGELOG.md. The SDK uses _NOTES but only puts a blurb in there, so it's not a changelog. But we include a changelog, so it makes sense to keep it named that way.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (7)
.changelog/README.md (3)
243-246: LGTM: Clear instructions for changelog updatesThe added instructions provide clear guidance on how to handle changelog updates and where specific files should exist. This helps maintain consistency and prevents confusion.
Consider adding a brief explanation of why changelog entry files should not be updated (e.g., "to preserve the original context of the changes"). This could help reinforce the importance of this instruction.
246-246: Consider consolidating redundant informationThe information about 'RELEASE_CHANGELOG.md' and version directories existing only in the .x branch is important. However, it's somewhat redundant with the content in the previous paragraph.
Consider consolidating this information into a single, clear statement to avoid repetition and improve readability. For example:
If you need to make tweaks or clarifications to the content, make the changes in the `RELEASE_CHANGELOG.md` file first, then copy/paste those into the `CHANGELOG.md` file. Do NOT update the changelog entry files. Note that the `RELEASE_CHANGELOG.md` file and `.changelog/<version>` directories should only exist on the `.x` branch, not on `main`, to maintain a clear separation between released and unreleased content.
Line range hint
1-300: LGTM: Comprehensive update to changelog management instructionsThe changes throughout the file consistently reflect the update from 'RELEASE_NOTES.md' to 'RELEASE_CHANGELOG.md' and provide clear, detailed instructions for both automated and manual changelog management processes. These updates align well with the PR objectives and should improve the overall changelog management workflow.
For future consideration, it might be beneficial to add a section on best practices for writing good changelog entries. This could include guidelines on clarity, conciseness, and how to effectively communicate the impact of changes to users and developers. This addition would further enhance the value of this documentation for contributors.
CONTRIBUTING.md (2)
Line range hint
36-70: LGTM: Enhanced contribution guidelinesThe expanded "Contributing" section provides a clearer step-by-step guide for contributors. The emphasis on issue tracking and collaboration, along with specific instructions for submitting pull requests, will help streamline the contribution process.
Consider adding a note about the importance of keeping pull requests focused on single issues to make the review process more manageable.
Line range hint
115-149: LGTM: Updated forking and dependency management instructionsThe revised "Forking" and "Dependencies" sections provide clearer and more up-to-date information for contributors. The instructions for managing forks and the mention of Go Modules for dependency management are particularly helpful.
Consider adding a brief explanation of why
go mod tidymight be necessary in case of broken builds due to third-party dependencies..changelog/get-dep-changes.sh (1)
Line range hint
345-350: Handle unexpected cases by exiting the script instead of adding a 'TODO' to the changelogIn the
elseblock starting at line 345, the script writes a 'TODO' entry into the changelog when an unexpected state is encountered. Adding such entries can cause confusion and should be avoided. Instead, consider displaying an error message and exiting the script to alert the user.Consider modifying the code as follows:
else # It shouldn't be possible to see this, but it's here just in case things go wonky. [[ -n "$verbose" ]] && printf '[%d/%d]: Unexpected state encountered.\n' "$i" "${#libs[@]}" - printf '* `%s` TODO: Could not identify dependency change details, fix me.\n' "$lib" >> "$changes" + printf 'Error: Could not identify dependency change details for `%s`.\n' "$lib" >&2 + exit 1 fiThis change ensures that unexpected cases are handled appropriately and prevents invalid or incomplete entries from being added to the changelog.
.changelog/prep-release.sh (1)
607-611: Prevent potential overwriting ofsummary.mdCopying
summary.mdback intounreleasedwithout checking may overwrite an existing summary. To avoid accidental data loss, consider verifying ifsummary.mdalready exists and handle it accordingly.Apply this diff to check for an existing
summary.mdbefore copying:if [[ -n "$v_rc" ]]; then # If this was an rc, copy the summary back into unreleased. [[ -n "$verbose" ]] && printf 'Copying summary.md back into unreleased.\n' + if [[ -f "$unreleased_sum_file" ]]; then + mv "$unreleased_sum_file" "${unreleased_sum_file}.backup" + [[ -n "$verbose" ]] && printf 'Existing summary.md backed up as summary.md.backup.\n' + fi cp "${new_ver_dir}/summary.md" "$unreleased_sum_file" || clean_exit 1 fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- .changelog/README.md (2 hunks)
- .changelog/combine-dep-lines.awk (1 hunks)
- .changelog/get-dep-changes.sh (1 hunks)
- .changelog/prep-release.sh (7 hunks)
- .changelog/unreleased/improvements/2181-combine-changelog-dep-lines.md (1 hunks)
- CONTRIBUTING.md (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changelog/unreleased/improvements/2181-combine-changelog-dep-lines.md
🧰 Additional context used
🔇 Additional comments (16)
.changelog/README.md (2)
233-234: LGTM: Improved file naming for release changelogThe change from 'RELEASE_NOTES.md' to 'RELEASE_CHANGELOG.md' is a good improvement. It makes the purpose of the file clearer and aligns well with the changelog management process.
250-251: LGTM: Updated manual instructions for consistencyThe change to refer to 'RELEASE_CHANGELOG.md' in the manual creation instructions maintains consistency with the earlier file name update. This ensures that both automated and manual processes are aligned.
CONTRIBUTING.md (3)
Line range hint
1-35: LGTM: Improved introduction and document structureThe expanded introduction and refined table of contents provide better context and navigation for contributors. This change enhances the overall usability of the document.
Line range hint
71-114: LGTM: Improved PR requirements and review processThe updated PR requirements and review process provide clear expectations for both contributors and reviewers. The inclusion of signed commits, approval requirements, and GitHub action checks enhances the overall quality control of contributions.
313-314: LGTM: Significantly improved release procedureThe expanded "Release Procedure" section provides a comprehensive guide for creating and managing releases. The addition of detailed steps for creating release branches, updating changelogs, and managing tags will lead to more consistent and well-documented releases.
The use of the
.changelog/prep-release.shscript to automate the changelog update process is a notable improvement. This should help ensure consistency in release documentation..changelog/combine-dep-lines.awk (4)
32-43: EnsureLibCountis reset appropriately inprintPrevLibfunctionThe
printPrevLibfunction resetsLibCountto zero after printing. This is correct, ensuring that the count starts fresh for each new library.
88-91: EnsureNewVeris updated correctly for each lineThe
NewVervariable is updated on every matching line, but in the context of combining entries, only the latest version should be used. Confirm thatNewVerreflects the most recent version after processing all entries for a library.Add a check to ensure
NewVerholds the latest version:# Within the else block where LibCount > 1 # Compare and update NewVer if necessary -if (/* condition to check if this NewVer is newer */) { + # Assuming a function compare_versions exists to compare semantic versions + if (compare_versions(NewVer, currentNewVer) < 0) { + NewVer = currentNewVer + } }You may need to implement a version comparison function to accurately determine the latest version.
100-102: Ensure all remaining libraries are processed in the END blockThe
printPrevLib()function is called in theENDblock to handle any remaining data. This is necessary for accurate processing and output.
47-47: Verify the regular expression used in theifconditionThe regular expression in the condition:
/^\* `.*` bumped to .* \(from .*\).* \[.*[[:digit:]]+\]\(.*\)/is intended to match lines that follow the expected bump line format. Ensure that this pattern accurately captures all possible variations of the input lines, including optional portions like warnings and forks.
Run the following script to test the regular expression against sample input lines:
This script helps confirm that the regular expression correctly identifies the intended lines.
✅ Verification successful
Regular expression verification successful
The regular expression accurately matches all intended bump line formats, including those with optional sections like warnings and forks.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Test the regular expression used in the AWK script # Sample input lines echo '* `lib1` bumped to v1.2.3 (from v1.2.2) [link1](https://example.com)' > test_input.txt echo '* `lib1` bumped to v1.2.4 (from v1.2.3) but is still replaced by <fork1> [link2](https://example.com)' >> test_input.txt echo '* `lib2` added at v0.4.5 [link3](https://example.com)' >> test_input.txt # Expected: Only lines matching the bump pattern should be printed awk '/^\* `.*` bumped to .* \(from .*\).* \[.*[[:digit:]]+\]\(.*\)/ {print}' test_input.txtLength of output: 516
.changelog/get-dep-changes.sh (1)
337-337: Addition of context comment improves maintainabilityThe added comment at line 337 clarifies that
combine-dep-lines.awkdepends on the specific formats used in this script. This provides valuable context for future maintainers, preventing accidental changes that could disrupt the release preparation process..changelog/prep-release.sh (6)
3-4: LGTM!The added comments appropriately document the script dependencies, enhancing the script's clarity.
101-107: LGTM!The script correctly defines the path to
combine-dep-lines.awk, allowing it to be overridden by theCOMBINE_DEP_LINES_AWKenvironment variable, and properly checks for its existence.
294-294: LGTM!The version validation logic for minor version increments appears correct and ensures sequential versioning.
466-490: LGTM!The code effectively cleans, sorts, and combines the entries of the dependencies section. This enhances the readability and organization of the dependency changelog entries.
520-526: LGTM!The script correctly recombines sections in the desired order, ensuring that the release notes are organized as intended.
553-554: LGTM!The variables for the new release notes file are correctly defined, facilitating the creation of the full release notes.
Description
This PR updates the
prep-release.shscript. It will now combine the version bump entries in the dependencies changelog section for a given library. It also fixes the script to properly createRELEASE_CHANGELOG.md.(instead ofRELEASE_NOTES.md) and updates our documentation to have the correct filename.E.g. before this PR, the
v1.20.0-rc1changelog entry had these lines:After this PR, it would instead have just this line:
This only combines "bumped to" lines (for now). It won't try to do anything with the "added at" or "removed at" entries (they still come through the way they did before).
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/).godoccomments..changelog/unreleased(see Adding Changes).Files changedin the Github PR explorer.Codecov Reportin the comment section below once CI passes.Summary by CodeRabbit
Release Notes
New Features
RELEASE_CHANGELOG.mdfor improved release note management.Improvements
Documentation
CONTRIBUTING.mdfile for improved clarity and navigation.