feat: add support for rendering .Body after .Subject as part of list #121
Conversation
There was a problem hiding this comment.
@masonkatz Thank you for the contribution!
Looks like you will need to rebase from master since we have pushed some updates.
Structurally, the code looks great. I think that I am following the intent behind this PR, but I feel that a resulting sample out put for reference would help. Would it be possible to provide a gist link to an example output demonstrating this feature?
Pull Request Test Coverage Report for Build 676911874
💛 - Coveralls |
|
@clok I updated (rebased to master) and created |
There was a problem hiding this comment.
Thank you for the updates and the test. Very helpful.
I am going to pull this down and do some testing on some of my repos that have more robust commit messages and squash commits. I want to check how this would impact those as I have put together some templates in the past to try and address those. I really like the simplicity of your solution.
One other thing on my mind is that we don't have robust documentation in the readme for the template options. It's an area for us to improve. Not a blocker for this PR, but I would appreciate your feedback on the Template documentation section and any ideas for improvement as a User of the tool.
Thanks again for the contribution. I'll circle back after my testing.
| @@ -461,3 +461,77 @@ func TestGeneratorWithTagFiler(t *testing.T) { | |||
| [Unreleased]: https://github.com/git-chglog/git-chglog/compare/v1.0.0...HEAD`, strings.TrimSpace(buf.String())) | |||
|
|
|||
| } | |||
|
|
|||
| func TestGeneratorWithTimmedBody(t *testing.T) { | |||
There was a problem hiding this comment.
Thank you for this. It was very helpful in demonstrating and testing the feature.
| - multi-line commit | ||
| More details about the change and why it went in. |
There was a problem hiding this comment.
I am interested to see what this would look like with squashed commits. I have templates that wrap them in preformat code blocks that has been effective, but with this feature it could be effective at exposing the squashed commits.
I am going to do some testing locally as I have many projects where squash commits are the norm.
|
Cool. Looking forward to your results and I'm happy to help build out the docs on options. |
|
This is really cool. It behaves how I would have hoped with the use of squash commits. What you end up with is a changelog that has the squashed commit messages exposed as an itemized list. Here is an example from a PR that was merged via a squash commit: GoodwayGroup/lib-hapi-good-tracer#5 The resulting squashed commit message Original output: https://github.com/GoodwayGroup/lib-hapi-good-tracer/blob/main/CHANGELOG.md#features-2 Regenerated using the template from your test set: https://gist.github.com/clok/10a272d955bbe9178f700d50ec9b0ede#features-2 While this may not be the desired usage for this feature, it is how I would have expected the Markdown to be generated with this feature in place. And I really like this option. I would like to see a Template Style added for the I don't want a feature like this to get lost. I think we need to update the docs or add the template to make sure our users can find this feature. Great work @masonkatz 🥇 |
|
@khos2ow good points about co-authors and sign-offs. I had not considered that. I could see that getting noisy as a result of this feature. We can even test on this repo history to see the effect. I agree that we have to consider those before moving forward. Would you recommend that we handle those similar to how we handle Mentions within the tool? |
Yes, I believe that would be the best way to handle this. Although for edge cases it might even be a good idea to provide some additional list of regex to exclude from trimmed body (maybe as part of the configuration.) |
|
@khos2ow That makes sense w/ the signed-off and co-authors lines. I just added support for both and can filter them out. When I create a changelog using the From the tests it looks like the intention is for notes to be either:
The code currently assumes everything in multi-line. Can I fix that? |
921a507 to
7b9408e
Compare
|
Updated to fix decreased code coverage, and make test better documentation of the change. |
When attempting to render a commit body below the summary line of the commit there are two problems: 1) The text needs to be indented two spaces to appear as part of the list. 2) Notes (e.g. BREAKING CHANGE) are included in the body and end up being repeating in a Notes section (if this is part of your template). To address git-chglog#1 add an `indent` func to the template parsing. To address git-chglog#2 add a `TrimmedBody` to the `Commit` fields. The `TrimmedBody` will include everything in `Body` but not any `Ref`s, `Note`s, `Mention`s, `CoAuthors`, or `Signers`. Both the CoAuthors and Signers are now first class in the Commit struct. With both of these a template block like: ``` {{ if .TrimmedBody -}} {{ indent .TrimmedBody 2 }} {{ end -}} ``` Will render the trimmed down body section as intended. See TestGeneratorWithTimmedBody for example of desired output.
|
Thanks for the help on the regex. I rebased again and squashed down to a single commit. |
There was a problem hiding this comment.
Looks solid. Thank you for the continued engagement on this feature. I did some more testing locally with different repos and commit situations. Great work @masonkatz
With the addition of TrimmedBody and CoAuthors I feel like we really need to update our documentation for the tool. Mentions are not clearly documented as well. Just wanted to note the opportunity here.
What does this do / why do we need it?
When attempting to render a commit body below the summary line of the commit there are two problems:
Users that want verbose changelogs will want the body of the commit to render as part of the unordered list element of the header.
How this PR fixes the problem?
To address 1 add an
indentfunc to the template parsing.To address 2 add a
TrimmedBodyto theCommitfields.The
TrimmedBodywill include everything inBodybut not anyRefs,Notes, orMentions.With both of these a template block like:
Will render the trimmed down body section as intended.
What should your reviewer look out for in this PR?
Check lists
Additional Comments (if any)
Both
commit_parser_test.goandfields.gohave whitespace diffs due to running gofmt. The actual diff on both is small.Which issue(s) does this PR fix?
none