Add git cliff toml to support generation of changelog #2697
Conversation
cliff.toml
Outdated
| * feat, feature | ||
| * imp | ||
| * bug, fix | ||
| * deprecated | ||
| * api-breaking | ||
| * state-machine-breaking, smb |
There was a problem hiding this comment.
these are arbitrary and can be decided by us. Note: these strings will not appear in the changelog entry, they are used to group the commits in the changelog.
There was a problem hiding this comment.
I recall some usage of a ! prefix which would indicate an API breaking change.
I'd like to suggest maybe adopting a ! prefix for api breaking changes and !! or something else for state machine breaking.
I feel like its common for us to use squash commits / PR titles similar to:
refactor: some refactor of a module or chore: cleaning up unused to code
But it might be nice to be able to prefix these like:
!refactor: some refactor that breaks an api or !!feat: this feat introduces a state machine breaking change.
Any thoughts on this?
chatton
requested review from
colin-axner,
AdityaSripal,
crodriguezvega,
charleenfei and
damiannolan
as code owners
|
I need to look a bit more into how this behaves when running from a release/vx.y.z branch to the next tag. |
cliff.toml
Outdated
| commit_preprocessors = [ | ||
| # A reference to an issue is appened to commits that looks like "(#1234)", this will be replaced | ||
| # with a link to that issue, e.g. "[#$1234](https://github.com/cosmos/ibc-go/issues/1234)". | ||
| { pattern = '\((\w+\s)?#([0-9]+)\)', replace = "([#${2}](https://github.com/cosmos/ibc-go/issues/${2}))" }, | ||
| # any reference to a pr like "pr-1234" will be replaced with a link to the PR. | ||
| { pattern = '\(pr-([0-9]+)\)', replace = "([#${1}](https://github.com/cosmos/ibc-go/pulls/${1}))" }, | ||
| ] |
There was a problem hiding this comment.
this effectively means I can write a commit message like
fix: fixed the problem in issue #1234 and also addressed concerns raised in pr-5678
And the issue and pr will be expanded into links in the changelog entry.
e.g.
Bug Fixes
There was a problem hiding this comment.
Will the commit merged automatically be linked?
There was a problem hiding this comment.
@colin-axner yep the commit being merged will be linked. This is the issue number in parens at the end of the commit in the generated output.
cliff.toml
Outdated
| * feat, feature | ||
| * imp | ||
| * bug, fix | ||
| * deprecated | ||
| * api-breaking | ||
| * state-machine-breaking, smb |
There was a problem hiding this comment.
I recall some usage of a ! prefix which would indicate an API breaking change.
I'd like to suggest maybe adopting a ! prefix for api breaking changes and !! or something else for state machine breaking.
I feel like its common for us to use squash commits / PR titles similar to:
refactor: some refactor of a module or chore: cleaning up unused to code
But it might be nice to be able to prefix these like:
!refactor: some refactor that breaks an api or !!feat: this feat introduces a state machine breaking change.
Any thoughts on this?
| @@ -85,6 +85,25 @@ All PRs require an approval from at least one CODEOWNER before merge. PRs which | |||
| - If you sat down with the PR submitter and did a pairing review please note that in the `Approval`, or your PR comments. | |||
| - If you are only making "surface level" reviews, submit any notes as `Comments` without adding a review. | |||
|
|
|||
| ### Commit Messages | |||
|
|
|||
| Commit messages should be [conventional](https://www.conventionalcommits.org/en/v1.0.0/). | |||
There was a problem hiding this comment.
Do all the commits in the PR need to follow this convention or the one that matters is the commit message that is entered when squashing the PR? Because in that case, we can have more relaxed rules for the commits while you're working on the PR, and then be careful when squashing the PR, which is something that we have control of.
There was a problem hiding this comment.
yes you're right it's only the final squashed commit that matters.
cliff.toml
Outdated
| commit_preprocessors = [ | ||
| # A reference to an issue is appened to commits that looks like "(#1234)", this will be replaced | ||
| # with a link to that issue, e.g. "[#$1234](https://github.com/cosmos/ibc-go/issues/1234)". | ||
| { pattern = '\((\w+\s)?#([0-9]+)\)', replace = "([#${2}](https://github.com/cosmos/ibc-go/issues/${2}))" }, | ||
| # any reference to a pr like "pr-1234" will be replaced with a link to the PR. | ||
| { pattern = '\(pr-([0-9]+)\)', replace = "([#${1}](https://github.com/cosmos/ibc-go/pulls/${1}))" }, | ||
| ] |
| @@ -85,6 +85,25 @@ All PRs require an approval from at least one CODEOWNER before merge. PRs which | |||
| - If you sat down with the PR submitter and did a pairing review please note that in the `Approval`, or your PR comments. | |||
| - If you are only making "surface level" reviews, submit any notes as `Comments` without adding a review. | |||
|
|
|||
| ### Commit Messages | |||
There was a problem hiding this comment.
Note to myself: add this to the PRs refactoring contributing.md.
Co-authored-by: Carlos Rodriguez <[email protected]>
There was a problem hiding this comment.
LGTM! Can we update the pr template as well to explain changelog entry usage? I'm still a little unclear about the usage with regards to auto merge. Would it be possible to just use the last line on a pr description for the changelog entry?
My main concern about entering the changelog entry when merging is typos/improper wording. It isn't uncommon to have requested changes to the changelog formatting, so it'd be nice if we could review the entry before merge
cliff.toml
Outdated
| commit_preprocessors = [ | ||
| # A reference to an issue is appened to commits that looks like "(#1234)", this will be replaced | ||
| # with a link to that issue, e.g. "[#$1234](https://github.com/cosmos/ibc-go/issues/1234)". | ||
| { pattern = '\((\w+\s)?#([0-9]+)\)', replace = "([#${2}](https://github.com/cosmos/ibc-go/issues/${2}))" }, | ||
| # any reference to a pr like "pr-1234" will be replaced with a link to the PR. | ||
| { pattern = '\(pr-([0-9]+)\)', replace = "([#${1}](https://github.com/cosmos/ibc-go/pulls/${1}))" }, | ||
| ] |
There was a problem hiding this comment.
Will the commit merged automatically be linked?
.github/PULL_REQUEST_TEMPLATE.md
Outdated
| @@ -12,18 +12,41 @@ are the most critical to review. | |||
|
|
|||
| closes: #XXXX | |||
|
|
|||
|
|
|||
| ### Commit Message / Changelog entry | |||
There was a problem hiding this comment.
cc @colin-axner @charleenfei @damiannolan @crodriguezvega
I updated the PR template to include a new section where contributors will include the proposed commit message / changelog entry. (can thumbs up if you're happy, and leave a comment if you have any suggestions / concerns! )
It is possible to either look at the first line of the commit for parsing, or to treat every line as a different commit, this type of behaviour isn't possible with this tool as far as I can tell. It does mean that as long as the first line is in a nice format, the rest can be in any format we like. cc @colin-axner |
| type: commit message | ||
| ``` | ||
|
|
||
| see the [guidelines](../CONTRIBUTING.md#commit-messages) for commit messages. (view raw markdown for examples) |
There was a problem hiding this comment.
Maybe move this into the comment section?
There was a problem hiding this comment.
this was intended to be short version with a prompt to look at the raw markdown (it might not be obvious if you don't know where to look).
Do you think it's not necessary to indicate that there is more details in comments?
There was a problem hiding this comment.
I was just thinking this would show on every pr description, but it is only relevant to the pr opener and thus it might make sense to make it a comment in the description so it isn't viewable by the reviewers. But I have no preference, we can always change it later
There was a problem hiding this comment.
I think it might provide some value, but like you said we can always remove it in the future if it just ends up being perceived as bloat!
Description
closes: #2106
Going forward, we can adhere to the conventional commit structure and ensure that every commit that gets merged to main is a descriptive changelog entry.
Currently this only generates a changelog of unreleased changes. If we want to maintain a static changelog file we can decide how we want to do that and implement in a follow up PR.
Update about release branches:
release/v5.1.xand then run(once this PR is backported)
With this example in particular, the output will also include changelog entries for 5.0.0 and 5.0.0-rc2. I suspect this has something to do with out v5.1.x branched from v5.0.x and are considered
unreleasedin some way. These are grouped into a separate category and can be safely ignored.All commits we wish to include must be conventional. Ie.
chore: something i did,fix: fixed a bug. If this format is not met, they will not be included in the changelog.We should ensure that all PRs include the proposed commit message (changelog entry) for approval in the PR. As there would not necessarily be a file that is reviewable in the Github UI.
E.g.
chore: integrating git cliff into the code base to automate generation of changelogsWe can start using this with the next set of releases as the current commit history will not generate a clean changelog.
The following was generated by running
Changelog
All notable changes to this project will be documented in this file.
[6.0.0] - 2022-11-07
Dependencies
Features
Bug Fixes
GetAppVersionin favour ofchannel.Version(#2302)Documentation
allow_messagesafter change DefaultParams on the host to allow all messages ("*") #2290 (#2367)hostkeeper.NewKeeperics27 integration docs (#2351)Testing
Miscellaneous Tasks
AllowUpdateAfterExpiry,AllowUpdateAfterMisbehaviour(#1843)AllowUpdateAfterProposal(#1940)Msgservice andRegisterAccountrpc boilerplate (#2068)sdk.Msgimpl for ics27MsgRegisterAccount(#2081)IsMiddlewareEnabledtoActiveChannelgenesis type (#2177)RegisterAccountrpc and msgs toRegisterInterchainAccount(#2253)SubmitTxtoSendTx(#2255)Refactor
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.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passes