Skip to content
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

feat(changelog): support generating changelog for different branches #808

Merged
merged 12 commits into from
Sep 9, 2024

Conversation

braineo
Copy link
Contributor

@braineo braineo commented Aug 22, 2024

Description

close #750

Motivation and Context

When a project needs to maintain diverged branches and create changelog for each branch this change is necessary.

for example a project's v3 is a complete rewrite and the changes onward have nothing to do which v2 branch. While v2 is in maintenance mode that the fixes are not relevant to v3 anymore.

when creating changelog for v3, it is necessary not to include the changes in the v2 which are not even included in the current branch

How Has This Been Tested?

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.

@braineo braineo requested a review from orhun as a code owner August 22, 2024 12:56
@braineo braineo marked this pull request as draft August 22, 2024 12:57
@braineo
Copy link
Contributor Author

braineo commented Aug 22, 2024

although the result of test-topo-order is not matching the expected.md but i feel like the new behavior should probably be the expected result.

the test commit first tags v0.1.0 and v0.2.0, then it checks out v0.1 and creates a new v0.1.1 tag.

in my opinion the expected full changelog should contain v0.1.0 and v0.1.1 instead of v0.2.0 only see #750 @chrissi-p

@codecov-commenter
Copy link

codecov-commenter commented Aug 22, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 23.07692% with 10 lines in your changes missing coverage. Please review.

Project coverage is 40.02%. Comparing base (08e761c) to head (bbb9617).

Files with missing lines Patch % Lines
git-cliff-core/src/repo.rs 33.34% 6 Missing ⚠️
git-cliff/src/lib.rs 0.00% 4 Missing ⚠️

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

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #808      +/-   ##
==========================================
- Coverage   40.13%   40.02%   -0.10%     
==========================================
  Files          20       20              
  Lines        1645     1657      +12     
==========================================
+ Hits          660      663       +3     
- Misses        985      994       +9     
Flag Coverage Δ
unit-tests 40.02% <23.08%> (-0.10%) ⬇️

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.

@braineo braineo marked this pull request as ready for review August 22, 2024 15:48
Copy link
Owner

@orhun orhun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! The issue is more clear to me now and I'm considering to switch to this new behavior. However, I need some explanations about how this is supposed to work :)

git-cliff-core/src/repo.rs Show resolved Hide resolved
@orhun
Copy link
Owner

orhun commented Aug 23, 2024

Also, any idea why the topo order fixture is failing? I'm trying to understand the change of behavior and its relation to the topo order 🤔

@braineo
Copy link
Contributor Author

braineo commented Aug 24, 2024

Also, any idea why the topo order fixture is failing? I'm trying to understand the change of behavior and its relation to the topo order 🤔

Because the tags chosen for the changelog are different now.

@braineo
Copy link
Contributor Author

braineo commented Aug 26, 2024

@orhun i've updated the description and the test cases. can you take a look at it plz?

@orhun
Copy link
Owner

orhun commented Aug 26, 2024

Well, I'm not fully convinced to go ahead with this. It breaks the topo order argument in a way that I don't understand.

Because the tags chosen for the changelog are different now.

Maybe you can expand this a bit?

@orhun
Copy link
Owner

orhun commented Aug 26, 2024

Maybe there is a way of supporting both cases if the commits are on other branches?

I'm not really sure if this fix is worth breaking the topo order argument.

@braineo
Copy link
Contributor Author

braineo commented Aug 26, 2024

Well, I'm not fully convinced to go ahead with this. It breaks the topo order argument in a way that I don't understand.

Because the tags chosen for the changelog are different now.

Maybe you can expand this a bit?

those features are independent from each other. My change only affects "what" commits are chosen, while leaving "how" they are sorted untouched.

See the chart below if it helps:

When one project has both v1 and v2 branches. It is typical that v1 branch change log does not include the changes from v2 since the code never exists in v1. And vice versa. The red dash areas are the tags considered for change log depending on at which commit the dev executes git-cliff

image

@braineo
Copy link
Contributor Author

braineo commented Aug 28, 2024

hi @orhun

is the explanation good enough or you are still looking into more details?

@orhun
Copy link
Owner

orhun commented Aug 30, 2024

those features are independent from each other. My change only affects "what" commits are chosen, while leaving "how" they are sorted untouched.

Okay I see now. And thanks for the diagram 🙂

How about we do something like this to support both cases?

self.should_include_tag(&head_commit, &commit)? || topo_order

@braineo
Copy link
Contributor Author

braineo commented Sep 2, 2024

those features are independent from each other. My change only affects "what" commits are chosen, while leaving "how" they are sorted untouched.

Okay I see now. And thanks for the diagram 🙂

How about we do something like this to support both cases?

self.should_include_tag(&head_commit, &commit)? || topo_order

@orhun maybe i don't fully understand the purpose of topo_order, it seems when not specified, it uses whatever order git2 returns for the tags.

it would be useful for any chosen set of the tags. with the suggested change, in order to use topo_order one must include all the tags in the changelog.

maybe you mean some configuration like filter_tags=false?

@orhun
Copy link
Owner

orhun commented Sep 2, 2024

it seems when not specified, it uses whatever order git2 returns for the tags.

Yeah, the tags are sorted by date as default.

with the suggested change, in order to use topo_order one must include all the tags in the changelog.

That's correct, we simply disable this new should_include_tag functionality with the code that I suggested above, since they work as expected together.

maybe you mean some configuration like filter_tags=false?

Do you mean adding a new configuration option?

@braineo
Copy link
Contributor Author

braineo commented Sep 3, 2024

with the suggested change, in order to use topo_order one must include all the tags in the changelog.

That's correct, we simply disable this new should_include_tag functionality with the code that I suggested above, since they work as expected together.

maybe you mean some configuration like filter_tags=false?

Do you mean adding a new configuration option

yes. i would suggest adding another option to toggle the tag filtering explicitly. because in the case when the tags are filtered, it is also desired to use the topo order but not the chronicle order. i still think filtering the tags and sorting the tags are independent from each other. with the suggested change, we will find two cases not available in the function matrix.

filter tags not filter tags
topo order
chronicle order

@braineo
Copy link
Contributor Author

braineo commented Sep 4, 2024

@orhun by separating the sort and filter option. it should be easier to add new sorting method like sort by semver version number etc. wdyt?

@orhun
Copy link
Owner

orhun commented Sep 4, 2024

yes. i would suggest adding another option to toggle the tag filtering explicitly

yeah, I think that's the most sensible thing to do for supporting both use cases at this point.

by separating the sort and filter option. it should be easier to add new sorting method like sort by semver version number etc. wdyt?

Do you mean something like:

sort_tags = "topo"
sort_tags = "date" # or chronically
sort_tags = "semver"

@braineo
Copy link
Contributor Author

braineo commented Sep 5, 2024

@orhun ok i added a new flag to opt-in.

Do you mean something like:

sort_tags = "topo"
sort_tags = "date" # or chronically
sort_tags = "semver"

yes, if in the future there are more options added to sorting, the change should not be breaking the tag filtering.

@braineo
Copy link
Contributor Author

braineo commented Sep 9, 2024

hi @orhun

what do you think of the latest changes?

@orhun orhun self-requested a review September 9, 2024 17:44
Copy link
Owner

@orhun orhun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the contribution & bearing with me :)

Applied some nitpicks and approved!

@orhun orhun changed the title feat: support generating change log for different branches feat(changelog): support generating changelog for different branches Sep 9, 2024
@orhun orhun merged commit 2a581a8 into orhun:main Sep 9, 2024
60 checks passed
@braineo braineo deleted the branch-aware branch September 9, 2024 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--unreleased does not behave as expected with tags on side branches
3 participants