search

orhun / git-cliff

A highly customizable Changelog Generator that follows Conventional Commit specifications ⛰️
https://git-cliff.org
Apache License 2.0
8.23k stars 165 forks source link

feat(changelog): make `changelog.header` a template #698

Closed Cyclonit closed 2 weeks ago

Cyclonit commented 2 weeks ago

Made changelog.header into a template like changelog.body and changelog.footer. Added test test-header-template.

Description

Implement #697.

Motivation and Context

Create parity between changelog.header, changelog.body and changelog.footer.

How Has This Been Tested?

Types of Changes

Checklist:

Cyclonit commented 2 weeks ago

Hi @orhun, functionally this is a very small change. I was able to copy the logic from the body and footer template processing 1-to-1. However, I do fear that we will have to flag this as a breaking change regardless. All of the tests and examples contain explicit trailing \n in the header. These are now obsolete. Without removing them, we would end up with an extra empty line between the header and the first release.

Cyclonit commented 2 weeks ago

I am not quite sure what to do about the allegedly wrong formatting in changelog.rs. When I run cargo fmt locally, it formats the code the way it is in this PR. I don't know why the pipeline demands it to be formatted differently. See https://github.com/orhun/git-cliff/actions/runs/9518131251/job/26238303168?pr=698

~Similarly I am having trouble figuring out why the test suite fails: https://github.com/orhun/git-cliff/actions/runs/9518131251/job/26238304948?pr=698 Actual and expected look identical to me.~ There were two errand tabs in the line after # Changelog. Took quite some time to notice them.

Cyclonit commented 2 weeks ago

And this is another problem: https://github.com/orhun/git-cliff/actions/runs/9518517603/job/26239610418?pr=698#step:3:842

I have no idea why the template behaves differently in test-header-template compared to test-footer-template. They are identical, but in test-header-template there is indentation, in test-footer-template there is not. I am done for today, maybe someone else could take a look into this.

codecov-commenter commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 72.72727% with 3 lines in your changes missing coverage. Please review.

Project coverage is 36.42%. Comparing base (dabe716) to head (17540a0). Report is 1 commits behind head on main.

Files Patch % Lines
git-cliff-core/src/changelog.rs 72.73% 3 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #698 +/- ## ========================================== + Coverage 36.29% 36.42% +0.13% ========================================== Files 19 19 Lines 1455 1461 +6 ========================================== + Hits 528 532 +4 - Misses 927 929 +2 ``` | [Flag](https://app.codecov.io/gh/orhun/git-cliff/pull/698/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Orhun+Parmaks%C4%B1z) | Coverage Δ | | |---|---|---| | [unit-tests](https://app.codecov.io/gh/orhun/git-cliff/pull/698/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Orhun+Parmaks%C4%B1z) | `36.42% <72.73%> (+0.13%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Orhun+Parmaks%C4%B1z#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

orhun commented 2 weeks ago

Can you resolve the conflicts? 😊

Cyclonit commented 2 weeks ago

I've fixed the tests and added the minimalist documentation update. How do we deal with the formatting? I cannot get my local rustfmt to format the code the way the pipeline wants it to be.

orhun commented 2 weeks ago

I've fixed the tests and added the minimalist documentation update. How do we deal with the formatting? I cannot get my local rustfmt to format the code the way the pipeline wants it to be.

Thanks! The formatting is already fixed in main, so if you merge those changes it will be fine I think.

I can also handle it before merging the PR, no biggie.

Cyclonit commented 2 weeks ago

I've fixed the tests and added the minimalist documentation update. How do we deal with the formatting? I cannot get my local rustfmt to format the code the way the pipeline wants it to be.

Thanks! The formatting is already fixed in main, so if you merge those changes it will be fine I think.

I can also handle it before merging the PR, no biggie.

I've tried improving the documentation by specifying the context of each template. Is this okay?

welcome[bot] commented 2 weeks ago

Congrats on merging your first pull request! ⛰️

Cyclonit commented 2 weeks ago

LGTM, thank you!

One thing that crossed my mind is that --prepend won't work as expected now when there are template variables in the header. It used to work like:

1- Read the contents from existing changelog file. 2- Replace the header in the file with the new changelog entries.

Now that the header is not a raw string & is not rendered, we won't be able to perform the replace properly.

Are you interested in fixing this as well? I think we only need to render header before replacing it in the file. I would really appreciate if you can test this case and create an issue!

I was not aware of this feature. I created a bug issue to deal with that: #712