-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch to unclog for the changelog. #2112
Conversation
…sdk.io/x/evidence v0.1.1
…/x/verygood at v0.7.1
…bft/cometbft-db to v0.9.6 (from v0.9.1).
…to to v1.6.1 (from v1.5.0).
…golang/protobuf to v1.5.7 (from v1.5.4).
…omethingelse at v0.12.3.
…om/bgentry/go-netrc at v0.0.0-20140422174119-9fd32a8b3d3d.
….com/cespare/xxhash/v3 at v3.0.2.
…48d20b. Bump github.com/cockroachdb/pebble to v1.1.2 (from v1.1.0).
… v1.3.0 (from v1.2.0).
…ithub.com/danieljoos/wincred to v1.3.1 (from v1.2.0).
…git diff output. It's not what I'll end up using, though, because it can't handle stuff like an entry moving between direct and indirect, and it can't consider the replace lines. But I'm keeping it just in case it's helpful.
…s being made to the go.mod file.
…r/nope => github.com/ours/nope v0.3.3
… to be confusing the runner.
… options instead of the non-portable -depth n option.
… that I've seen it fail.
…eeded because we'd try to look up the current branch, which might be HEAD in some cases that indicates not having some git stuff available. But that's gone now, so we're good here too.
…ies. None of them should have sub-lists. And this way, a leading space won't throw things off.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (4)
.changelog/dependabot-changelog.sh (1)
141-145
: Improve error message for unknown title format.The error message for an unknown title format should provide more context to help users understand the expected format.
- printf 'Unknown title format: %s\n' "$title" + printf 'Unknown title format: %s. Expected format: "Bump <library> from <old version> to <new version>"\n' "$title".changelog/README.md (1)
30-44
: Fix example command formatting.The example command for adding changes should be formatted consistently.
- $ unclog add --pull-request 123 --section bug-fixes --id fix-the-thing --message 'Fix the thing that was broken' + $ unclog add --pull-request 123 --section bug-fixes --id fix-the-thing --message "Fix the thing that was broken".changelog/get-dep-changes.sh (2)
128-142
: Improve error message for missing PR or issue number.The error message for missing PR or issue number should provide more context to help users understand the requirement.
- printf 'You must provide either a --pr <num> or --issue <num>.\n' + printf 'You must provide either a --pr <num> or --issue <num>. Both cannot be provided simultaneously.\n'
145-160
: Optimize verbose output for cleaned ID.The verbose output for the cleaned ID should be consistent with other verbose outputs.
- [[ -n "$verbose" ]] && printf 'Cleaned id: %s\n' "'$id'" + [[ -n "$verbose" ]] && printf 'Cleaned ID: "%s"\n' "$id"
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (15)
- .changelog/README.md (1 hunks)
- .changelog/change-template.md (1 hunks)
- .changelog/config.toml (1 hunks)
- .changelog/dependabot-changelog.sh (1 hunks)
- .changelog/get-dep-changes.sh (1 hunks)
- .changelog/get-valid-sections.sh (1 hunks)
- .changelog/lint-unreleased.sh (1 hunks)
- .changelog/prep-release.sh (1 hunks)
- .changelog/unreleased/improvements/2112-switch-to-unclog.md (1 hunks)
- .github/workflows/changelog.yml (2 hunks)
- .github/workflows/lint.yml (2 hunks)
- CHANGELOG.md (3 hunks)
- CONTRIBUTING.md (7 hunks)
- Makefile (2 hunks)
- scripts/update-toc.sh (1 hunks)
Files skipped from review due to trivial changes (2)
- .changelog/change-template.md
- .changelog/unreleased/improvements/2112-switch-to-unclog.md
Additional context used
LanguageTool
CONTRIBUTING.md
[style] ~91-~91: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... approved by two or more maintainers. - It must pass all required GitHub action ch...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~223-~223: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...d branches must not failmake lint
. - Protected branches should not fail any GitHub act...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~332-~332: This sentence should probably be started with a verb instead of the noun ‘PR’. If not, consider inserting a comma for better clarity.
Context: ...ew release page to be created. #### 4. PR the .x Branch Back to Main This PR sho...(SENT_START_NN_DT)
Additional comments not posted (55)
.github/workflows/lint.yml (1)
43-45
: New job for linting unreleased changelog content looks good.The new job aligns well with the existing workflow structure and follows best practices.
.changelog/lint-unreleased.sh (3)
1-11
: Initial setup and directory existence check look good.The script correctly sets up the environment and handles the absence of the unreleased directory.
13-32
: Function to check valid sections and iteration logic look good.The function correctly identifies valid sections and the iteration logic is sound.
34-50
: Invalid filename identification and error printing look good.The script correctly identifies invalid filenames and prints errors as expected.
.changelog/get-valid-sections.sh (2)
1-15
: Initial setup and file existence check look good.The script correctly sets up the environment and handles the absence of the
CHANGELOG.md
file.
16-24
: Section extraction usingawk
looks good.The
awk
script correctly extracts valid sections and handles edge cases..github/workflows/changelog.yml (4)
18-18
: Conditional execution clause looks good.The conditional execution clause ensures that the
changelog
job runs only for PRs labeled 'dependencies', optimizing resource usage.
20-25
: Environment variables enhance security.Defining environment variables for PR metadata prevents injection vulnerabilities by avoiding direct access to the PR title within the script.
34-35
: Fetching target branch step looks good.The step fetches the target branch associated with the PR, ensuring the script has the necessary context.
37-44
: Changelog entry generation step looks good.The step generates a changelog entry based on the PR details, enhancing automation.
.changelog/config.toml (12)
1-7
: Project URL setting looks good.The project URL is correctly set to the Provenance project's GitHub repository.
9-14
: Change template setting looks good.The change template is set to
change-template.md
, ensuring a consistent format for changes added through the CLI.
16-25
: Wrap length setting looks good.The wrap length is set to 200 characters, accommodating the length of issue/PR links and providing sufficient space for main content.
27-28
: Changelog heading setting looks good.The heading is set to
# CHANGELOG
, providing a clear and consistent title for the changelog.
30-32
: Bullet style setting looks good.The bullet style is set to
*
, ensuring consistency in list formatting.
34-35
: Empty message setting looks good.The empty message provides a clear prompt to add entries when the changelog has no entries.
37-40
: Epilogue filename setting looks good.The epilogue filename is set to
epilogue.md
, allowing for additional content to be appended to the changelog.
42-51
: Release sorting setting looks good.Releases are sorted by version, ensuring a logical order for release entries.
53-64
: Release date formats setting looks good.The release date formats include common date formats, ensuring correct parsing of release dates.
67-75
: Unreleased entries settings look good.The settings for unreleased entries, including folder name and heading, are correctly configured.
78-87
: Change sets settings look good.The settings for change sets, including summary filename and entry extension, are correctly configured.
90-97
: Change set sections settings look good.Entries are sorted by ID, ensuring a logical order for entries within change sets.
scripts/update-toc.sh (1)
140-140
: Temporary filename generation looks good.Using the variable
$filename
for generating the temporary filename enhances the function's flexibility..changelog/dependabot-changelog.sh (2)
5-25
: Usage instructions are clear and well-structured.The
show_usage
function provides clear and comprehensive usage instructions for the script.
185-202
: Fix verbose output for repository root.The verbose output should reference the correct variable for the repository root.
- [[ -n "$verbose" ]] && printf ' Repo Root: "%s"\n' "$repo_root" + [[ -n "$verbose" ]] && printf ' Repo Root: "%s"\n' "$repo_root"Likely invalid or redundant comment.
.changelog/README.md (6)
1-27
: Overview section is clear and well-written.The Overview section provides a clear and concise summary of the changelog management process.
73-85
: Dependencies section is clear and well-written.The Dependencies section provides clear instructions for creating changelog entries for dependency changes.
88-111
: Section Names section is clear and well-written.The Section Names section provides clear instructions for retrieving valid section names.
114-134
: Unclog section is clear and well-written.The Unclog section provides clear instructions for using
unclog
to manage changelog entries.
137-155
: Entry Files section is clear and well-written.The Entry Files section provides clear instructions for creating and managing entry files.
159-224
: Releases section is clear and well-written.The Releases section provides clear instructions for preparing and managing releases.
.changelog/get-dep-changes.sh (2)
5-53
: Usage instructions are clear and well-structured.The
show_usage
function provides clear and comprehensive usage instructions for the script.
56-126
: Argument parsing is correctly implemented.The argument parsing block correctly handles various arguments and provides appropriate error messages for invalid inputs.
Makefile (3)
280-280
: Approved: Addition of.changelog/lint-unreleased.sh
tolint
target.The addition of the new linting script enhances code quality checks.
305-305
: Approved: Update toupdate-tocs
target.Including
.changelog/README.md
in theupdate-tocs
target enhances documentation coherence.
307-309
: Approved: Addition ofget-valid-sections
target.The new target enhances changelog management by validating sections.
CONTRIBUTING.md (3)
49-49
: Approved: Correction of "Github" to "GitHub".The correction adheres to branding standards.
91-91
: Approved: Rephrasing of PR requirements.The rephrasing improves readability and clarity.
Tools
LanguageTool
[style] ~91-~91: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... approved by two or more maintainers. - It must pass all required GitHub action ch...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
223-223
: Approved: Rephrasing of the release procedure.The rephrasing improves readability and clarity.
Tools
LanguageTool
[style] ~223-~223: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...d branches must not failmake lint
. - Protected branches should not fail any GitHub act...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
.changelog/prep-release.sh (10)
1-65
: Approved: Usage function and argument parsing.The usage function and argument parsing are correctly implemented.
67-126
: Approved: Version validation.The version validation is correctly implemented.
157-210
: Approved: Date validation.The date validation is correctly implemented.
213-235
: Approved: Temporary directory creation and cleanup.The temporary directory creation and cleanup are correctly implemented.
237-250
: Approved: Version handling.The version handling is correctly implemented.
252-290
: Approved: Existing versions validation.The existing versions validation is correctly implemented.
298-376
: Approved: Release candidate combination.The release candidate combination is correctly implemented.
388-538
: Approved: New release notes creation.The new release notes creation is correctly implemented.
544-564
: Approved: New changelog creation.The new changelog creation is correctly implemented.
570-584
: Approved: Finalization.The finalization is correctly implemented.
CHANGELOG.md (6)
13-14
: Clear and Accurate InstructionsThe instructions for using the
unclog
application to create changelog entry files are clear and accurate.
15-19
: Example and Details are CorrectThe example of adding an entry using
unclog
and the details about the file creation are correct and easy to follow.
21-25
: Clear and Accurate InstructionsThe instructions for generating dependency changelog entries using the
get-dep-changes.sh
script are clear and accurate.
27-27
: Clear Note on PrecedenceThe note about the precedence of the
CHANGELOG.md
file over the.changelog
directory in case of discrepancies is clear and provides the necessary information.
29-33
: Clear Guidelines for Including Messages and LinksThe guidelines for including messages and links to issues or pull requests in changelog entries are clear and easy to follow.
47-49
: Clear and Accurate Information on Valid--section
ValuesThe information on valid
--section
values and how to get a list of valid values is clear and accurate.
…endabot changelog script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .changelog/dependabot-changelog.sh (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .changelog/dependabot-changelog.sh
…than any change in .changlelog, now only trigger on a change to the lint script or the unreleased dir.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/lint.yml (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/lint.yml
The |
…f adding directly to CHANGELOG.md.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/PULL_REQUEST_TEMPLATE.md (1 hunks)
Files skipped from review due to trivial changes (1)
- .github/PULL_REQUEST_TEMPLATE.md
Good call. I've tweaked it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/PULL_REQUEST_TEMPLATE.md (1 hunks)
Files skipped from review due to trivial changes (1)
- .github/PULL_REQUEST_TEMPLATE.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some seriously complex scripting in here... seems like it should work out ok but the proof will come with use.
Yeah. Luckily, both And The About the only thing that concerns me a little is the post-release PR to update the changelog on |
Description
This PR will switch us to using unclog for our unreleased changelog information.
After this PR is merged, you should no longer add new entries to the
CHANGELOG.md
file, but instead create new unreleased entry files usingunclog
.Example:
If your change involves dependency changes, you should utilize the
.changelog/get-dep-changes.sh
script (instead ofunclog
) to generate the dependency entry file (more info below). This should be in addition to one or more entries specific to your change.Example:
To get a list of valid
--section
values, runmake get-valid-sections
(or.changelog/get-valid-sections.sh
).Example:
A new linter script has been added (
.changelog/lint-unreleased.sh
) that fails if there's an unexpected section directory inunreleased
or if there are any files under that dir with a space in the filename. Themake lint
target now also runs this linter, as well as the GitHub action.We'll still use the
CHANGELOG.md
as a source of truth though;unclog
is only being used to manage unreleased dependencies and generate new release content. We won't be using it to create the entirety of the changelog. As such, when updating theCHANGELOG.md
file onmain
, we'll also be deleting the associatedunreleased
entry files instead of moving them to a version directory.For more thorough dependency change entries, I've created a script to analyze changes being made to
go.mod
and generate all the appropriate dependency changelog entries:.changelog/get-dep-changes.sh
. This script is designed for you to use. It will only identify dependency changes involving thego.mod
file. The dependabot changelog action utilizes a newscripts/dependabot-changelog.sh
which callsget-dep-changes.sh
but if there are no changes togo.mod
, it then creates an appropriate changelog entry based off the title of the PR.Before this PR, if dependabot bumped one library that caused others to be bumped (e.g. when doing
go mod tidy
), the changelog only had an entry for the library that initiated the PR. Now, though, we'll have a more detailed listing of the changes.Further, the format of the entries have changed a little bit. Entries now have the format
<library> <action> <version> [(from <old version>)]
. The<action>
will be one ofbumped to
,added at
, orremoved at
. The(from <old version>)
part is only included for version bumps. If the library was removed, the<version>
is what we were using right before we removed the dependency. Replace lines are now considered for these entries too. If a replace line is involved, the<version>
and<old version>
strings can also contain the replacing library.Whenever your PR involves a
go.mod
change, you should runscripts/get-dep-changes.sh
to generate the changelog entries (instead of usingunclog
for the dependency changes).If your change contains a library bump, but that bump isn't the purpose of the change, you should create both a
dependencies
entry (using this script) and an entry more specific to your change (usingunclog
).I tested the github action in this PR by doing the following:
go.mod
: d781915."Bump some/thing from v1.2.3 to v1.2.4"
.go.mod
back to whatmain
has and pushed that up: b91decd.I've also created a new script that will make the updates that are needed to mark a new release:
.changelog/prep-release.sh
. This script will create/update theRELEASE_NOTES.md
file, add the version to theCHANGELOG.md
file, and move the entry files out of.changelog/unreleased/
.This script handles the following cases (at the very least):
I've updated the
CONTRIBUTING.md
release procedure documentation to include use of this script.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/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passesSummary by CodeRabbit
New Features
Documentation
CHANGELOG.md
and.changelog/README.md
to clarify the changelog process and entry guidelines.CONTRIBUTING.md
for clarity and consistency in instructions.Bug Fixes