fix(changelog): fix previous version links #364

tvcsantos · 2023-12-03T09:11:43Z

Description

In this PR the main goal was to simplify and fix the previous version links on all scenarios.

Motivation and Context

There were some scenarios where the previous version link was incorrect. For example

sh test-fixtures-locally.sh test-keep-a-changelog-links-current-arg --current

was producing the wrong link for the current tag, considering it as the only release.

How Has This Been Tested?

Created several extra tests for fixtures to address the several combinations and provide a strong test suite to avoid regressions further on the line.

Screenshots / Logs (if applicable)

Types of Changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation (no code change)
Refactor (refactoring production code)
Other

Checklist:

My code follows the code style of this project.
I have updated the documentation accordingly.
I have formatted the code with rustfmt.
I checked the lints with clippy.
I have added tests to cover my changes.
All new and existing tests passed.

codecov-commenter · 2023-12-03T09:14:29Z

Codecov Report

Attention: 18 lines in your changes are missing coverage. Please review.

Comparison is base (ff2881b) 42.73% compared to head (c55537d) 41.98%.

Files	Patch %	Lines
git-cliff/src/lib.rs	0.00%	18 Missing ⚠️

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #364      +/-   ##
==========================================
- Coverage   42.73%   41.98%   -0.75%     
==========================================
  Files          13       13              
  Lines         667      679      +12     
==========================================
  Hits          285      285              
- Misses        382      394      +12

Flag	Coverage Δ
unit-tests	`41.98% <0.00%> (-0.75%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

tvcsantos · 2023-12-03T09:19:16Z

Note the first pipeline run is on purpose, and follows a TDD approach, so that we can see the current failing scenarios.

orhun · 2023-12-03T16:45:57Z

Hey, thanks for creating this! 🐻

I didn't quite get which issue this PR is solving. Can you elaborate what's the problem before and what was your approach for solving it? Giving example input/output would be really helpful since it is kind of difficult to infer it from the code.

tvcsantos · 2023-12-03T16:58:15Z

Hey, thanks for creating this! 🐻

I didn't quite get which issue this PR is solving. Can you elaborate what's the problem before and what was your approach for solving it? Giving example input/output would be really helpful since it is kind of difficult to infer it from the code.

Sure

With the current code before the PR with the following commits

GIT_COMMITTER_DATE="2021-01-23 01:23:45" git commit --allow-empty -m "feat: add feature 1"
git tag v0.1.0

GIT_COMMITTER_DATE="2021-01-24 01:23:46" git commit --allow-empty -m "feat: add feature 2"
git tag v0.2.0

Running

git-cliff --current

will result in a scenario where we won't have a previous version correctly set for v0.2.0.

(you can check this here https://github.com/orhun/git-cliff/actions/runs/7075903455/job/19258554935#step:3:569)

After this PR this is correctly fixed and the previous tag v0.1.0 is set.

(you can check it here https://github.com/orhun/git-cliff/actions/runs/7076145622/job/19259077413#step:3:573)

Ty, if you need something else ping me :)

tvcsantos · 2023-12-03T17:04:11Z

Also a bit more in detail on the approach to solve the previous links in a uniform approach. The idea was the following:

Find the first tag that is processed in the releases loop.
Get the tag before this one, that will in the end always in any scenario be the previous of the first release.
If no tag is ever processed then we are seeing commits after the last tag, hence previous is set to the last tag.

…-wrong-previous-version

tvcsantos · 2023-12-03T17:52:47Z

Btw just merged with latest changes on main. If you need I can rebase everything to avoid the merge commit (I guess that if you are using squash you don't need it, but please do tell me if it is required).

orhun · 2023-12-04T19:19:07Z

I think there is a general confusion about the usage of --latest vs --current. I tried to explain the use-case for --current here: https://git-cliff.org/blog/git-cliff-0.5.0#--current

Also see #37 for the implementation details.

So in your case, if you want the previous release to be set, you can simply use --latest:

$ git cliff --context --latest | jq .[].previous

{
  "version": "v0.1.0",
  "commits": [],
  "commit_id": "0b5b2e2cb8c5e921e1c34b0ba60e442255d2a659",
  "timestamp": 0,
  "previous": null
}

I hope this helps, let me know! I think I can update the docs about clarifying the usage of --current / happily accept a PR about that! 🐻

orhun · 2023-12-06T18:30:24Z

ping 🐻

tvcsantos · 2023-12-06T20:57:27Z

Hey hey sorry for the delayed response.

Okay so let me use this example

suppose that I have 3 tags

v1.0.0
v2.0.0
v3.0.0

And now at this moment I'm on tag v3.0.0. What is the previous version of v3.0.0? It's v2.0.0

From what I got If I use git-cliff --current previous should not be set, is that it?!

orhun · 2023-12-07T18:50:06Z

And now at this moment I'm on tag v3.0.0. What is the previous version of v3.0.0? It's v2.0.0
From what I got If I use git-cliff --current previous should not be set, is that it?!

I think there is a misunderstanding. What I'm saying is simply there is no need to use --current if you are already on the latest tag (v3.0.0), you should use --latest instead which correctly sets the previous version.

However, if you check out v2.0.0 for example and use --current, then previous version should be set of course. Is this what this PR is fixing?

git-cliff/src/lib.rs

.github/fixtures/test-keep-a-changelog-links-latest-arg/cliff.toml

tvcsantos · 2023-12-08T19:10:06Z

And now at this moment I'm on tag v3.0.0. What is the previous version of v3.0.0? It's v2.0.0
From what I got If I use git-cliff --current previous should not be set, is that it?!

I think there is a misunderstanding. What I'm saying is simply there is no need to use --current if you are already on the latest tag (v3.0.0), you should use --latest instead which correctly sets the previous version.

However, if you check out v2.0.0 for example and use --current, then previous version should be set of course. Is this what this PR is fixing?

Sorry for the delay on the response. I've tried to be thorough and did a in depth analysis to check that I didn't messed up anything. I've documented it here https://github.com/tvcsantos/git-cliff-test. And yes the PR is fixing the last issue you mentioned and with that also makes the code uniform for all cases, without requiring special ifs and the cumbersome check for tag lens.

Extracting the example from my test that is it:

diff --color -r results/1_4_0/at_2_0_0/context/current.json results/new/at_2_0_0/context/current.json
31c31
<       "version": null,
---
>       "version": "v1.0.0",
33c33
<       "commit_id": null,
---
>       "commit_id": "fa211a975e7c2a60053561e45660b0c57b847c72",
diff --color -r results/1_4_0/at_2_0_0/render/current.md results/new/at_2_0_0/render/current.md
1c1
< ## [2.0.0](https://github.com/tvcsantos/git-cliff-test/releases/tag/v2.0.0) - 2023-12-08
---
> ## [2.0.0](https://github.com/tvcsantos/git-cliff-test/compare/v1.0.0...v2.0.0) - 2023-12-08

orhun · 2023-12-09T12:28:44Z

And yes the PR is fixing the last issue you mentioned and with that also makes the code uniform for all cases, without requiring special ifs and the cumbersome check for tag lens.

That's nice! I guess we can confirm that the behavior isn't changed from the fixture tests as well.

orhun · 2023-12-09T12:30:31Z

There are some conflicts with main, can you fix those and address the review comments for proceeding with this? 🐻

…-wrong-previous-version

tvcsantos · 2023-12-09T15:46:21Z

All updated and merged with the last changes from main. Ty 💚

tvcsantos · 2023-12-09T16:56:52Z

All seems good except for the failing "Continuous Integration / Links (pull_request)" that I think is not related. I've checked the failure and the links seem correct on the CHANGELOG 🤔

orhun

Last comment 🐻

git-cliff/src/lib.rs

orhun

Thanks for your contribution! 💖

test: add more tests for previous links

5801300

fix: fix wrong previous version

a6f36fc

tvcsantos force-pushed the bugfix/fix-wrong-previous-version branch from b333bd8 to a6f36fc Compare December 3, 2023 10:03

test: fix wrong tag on test fixture

6a6e69a

tvcsantos marked this pull request as ready for review December 3, 2023 11:44

tvcsantos requested a review from orhun as a code owner December 3, 2023 11:44

tvcsantos changed the title ~~fix: fix previous version links~~ fix(changelog): fix previous version links Dec 3, 2023

tvcsantos added 2 commits December 3, 2023 17:47

Merge branch 'main' of github.com:tvcsantos/git-cliff into bugfix/fix…

1a85682

…-wrong-previous-version

test: add fixture for bump with keep a changelog links

a7a96dd

orhun reviewed Dec 7, 2023

View reviewed changes

git-cliff/src/lib.rs Outdated Show resolved Hide resolved

orhun reviewed Dec 7, 2023

View reviewed changes

.github/fixtures/test-keep-a-changelog-links-latest-arg/cliff.toml Outdated Show resolved Hide resolved

tvcsantos added 3 commits December 9, 2023 15:36

Merge branch 'main' of github.com:tvcsantos/git-cliff into bugfix/fix…

31850ce

…-wrong-previous-version

perf: avoid setting previous if already set

b0d31a5

style: remove unused comment

01992d9

tvcsantos added 2 commits December 9, 2023 16:26

refactor: change code style for readability

a3f6503

fix: fix compilation failure

e9d32fd

orhun reviewed Dec 9, 2023

View reviewed changes

git-cliff/src/lib.rs Show resolved Hide resolved

mergify bot and others added 4 commits December 9, 2023 20:18

Merge branch 'main' into bugfix/fix-wrong-previous-version

77d5202

Merge branch 'main' into bugfix/fix-wrong-previous-version

a6c8a6a

docs: update with comments

7684e7a

style: reformat code

c55537d

orhun approved these changes Dec 10, 2023

View reviewed changes

orhun merged commit 44c93b7 into orhun:main Dec 10, 2023
39 of 40 checks passed

tvcsantos deleted the bugfix/fix-wrong-previous-version branch December 10, 2023 17:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(changelog): fix previous version links #364

fix(changelog): fix previous version links #364

tvcsantos commented Dec 3, 2023 •

edited

Loading

codecov-commenter commented Dec 3, 2023 •

edited

Loading

tvcsantos commented Dec 3, 2023

orhun commented Dec 3, 2023

tvcsantos commented Dec 3, 2023 •

edited

Loading

tvcsantos commented Dec 3, 2023 •

edited

Loading

tvcsantos commented Dec 3, 2023

orhun commented Dec 4, 2023

orhun commented Dec 6, 2023

tvcsantos commented Dec 6, 2023 •

edited

Loading

orhun commented Dec 7, 2023

tvcsantos commented Dec 8, 2023 •

edited

Loading

orhun commented Dec 9, 2023

orhun commented Dec 9, 2023

tvcsantos commented Dec 9, 2023

tvcsantos commented Dec 9, 2023

orhun left a comment

orhun left a comment

fix(changelog): fix previous version links #364

fix(changelog): fix previous version links #364

Conversation

tvcsantos commented Dec 3, 2023 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Screenshots / Logs (if applicable)

Types of Changes

Checklist:

codecov-commenter commented Dec 3, 2023 • edited Loading

Codecov Report

tvcsantos commented Dec 3, 2023

orhun commented Dec 3, 2023

tvcsantos commented Dec 3, 2023 • edited Loading

tvcsantos commented Dec 3, 2023 • edited Loading

tvcsantos commented Dec 3, 2023

orhun commented Dec 4, 2023

orhun commented Dec 6, 2023

tvcsantos commented Dec 6, 2023 • edited Loading

orhun commented Dec 7, 2023

tvcsantos commented Dec 8, 2023 • edited Loading

orhun commented Dec 9, 2023

orhun commented Dec 9, 2023

tvcsantos commented Dec 9, 2023

tvcsantos commented Dec 9, 2023

orhun left a comment

Choose a reason for hiding this comment

orhun left a comment

Choose a reason for hiding this comment

tvcsantos commented Dec 3, 2023 •

edited

Loading

codecov-commenter commented Dec 3, 2023 •

edited

Loading

tvcsantos commented Dec 3, 2023 •

edited

Loading

tvcsantos commented Dec 3, 2023 •

edited

Loading

tvcsantos commented Dec 6, 2023 •

edited

Loading

tvcsantos commented Dec 8, 2023 •

edited

Loading