feat(git): disable sorting commits topologically (#804) #1121

SebClapie · 2025-03-26T10:27:09Z

Description

Add sorting option for commits

Motivation and Context

This change is related to #804
When I tried git-cliff on my personal git repository, the commits order was not correct.
I finally found why the order was not correct. The CHANGELOD generated got the same order than the command git log --oneline --topo-order.

How Has This Been Tested?

I did not add a specific test for this feature, I apologize, I just installed rust for the first time yesterday and I never used GitHub before ..
I tried to follow your recommandation.
I launched:

cargo build
cargo test
cargo fmt
cargo clippy

I also tested on my side that the sort is functional on my repro. I tested the command line option --disable-topo-order-commits
I also tested this option inside my file cliff.toml

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 launched cargo fmt and cargo clippy
I have updated the documentation accordingly.
Maybe the text can be improved
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.

welcome · 2025-03-26T10:27:12Z

Thanks for opening this pull request! Please check out our contributing guidelines! ⛰️

codecov-commenter · 2025-03-26T11:11:38Z

Codecov Report

Attention: Patch coverage is 22.22222% with 7 lines in your changes missing coverage. Please review.

Project coverage is 41.48%. Comparing base (9e4bd07) to head (25b5c37).

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1121      +/-   ##
==========================================
- Coverage   41.58%   41.48%   -0.10%     
==========================================
  Files          21       21              
  Lines        1816     1823       +7     
==========================================
+ Hits          755      756       +1     
- Misses       1061     1067       +6

Flag	Coverage Δ
unit-tests	`41.48% <22.23%> (-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.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

orhun

Thanks for the PR! This is definitely one of the crucial missing pieces so I appreciate you taking time to implement it.

Some comments:

I think the option should be called topo_order_commits. disable_topo_order_commits does not read well since we are using a boolean. (e.g. disable disable_topo_order_commits)
We already have a topo_order option for sorting the tags. I think we should rename that to topo_order_tags and deprecate the old option. What do you think? (cc. @Cyclonit since he worked on revamping the config before)
We should add fixture tests for this, see the instructions here.

Cyclonit · 2025-03-27T10:57:41Z

I would propose refactoring the system to have two options:

git.commit_order_by = topology/time
changelog.release_order_by = topology/time

Having enums instead of booleans improves readability significantly. But I know @orhun likes using as few words as possible ;)

However, we do not have any way of soft deprecating old config options and adding new ones beside it. That is something that would have to be designed.

SebClapie · 2025-03-27T11:43:37Z

Hello,

Thanks @orhun for your feedback.
I will a look to the fixture, and I may come back to you if I need help

I can rename disable_topo_order_commits to topo_order_commits.
Or do I look for @Cyclonit proposal ?

git.commit_order_by = topology/time

orhun · 2025-03-28T03:19:25Z

I would propose refactoring the system to have two options:
* `git.commit_order_by` = `topology`/`time`

* `changelog.release_order_by` = `topology`/`time`

In that case we also need to deprecate the old options. I'd prefer we go with topo_order_commits for now and revamp the configuration later.

However, we do not have any way of soft deprecating old config options and adding new ones beside it. That is something that would have to be designed.

We can probably add a warning while parsing the config 🤔 I'll look into it.

LMK @SebClapie when this is ready for review.

SebClapie · 2025-03-31T06:45:19Z

Hello @orhun ,

I updated the code to rename the option topo_order_commits , and the default value is true to keep previous behavior
I added a new test fixture, but I don't know why I got this issue "line 2: ./commit.sh: Permission denied". I follow your procedure for add a new ficture, and it works locally for me.

Cyclonit · 2025-03-31T12:40:52Z

Hello @orhun ,

... I added a new test fixture, but I don't know why I got this issue "line 2: ./commit.sh: Permission denied".

The commit.sh you created does not have POSIX permissions assigned to it. This usually happens because you created the file on windows. Run this to fix it, commit and push again:

git update-index --chmod=+x <file>

.github/fixtures/test-topo-order-commits/cliff.toml

SebClapie · 2025-03-31T14:37:59Z

The Typos action failed. But I don't think it's linked to my modification
I don't know how to retrigger it ....

config/cliff.toml

orhun

LGTM, some minor comment issues :)

Can you also create a new issue for tracking the deprecation of topo_order (in favor of topo_order_tags)?

website/docs/configuration/git.md

Co-authored-by: Orhun Parmaksız <orhunparmaksiz@gmail.com>

orhun

Some styling suggestions

.github/fixtures/test-topo-order-commits/cliff.toml

website/docs/configuration/git.md

Co-authored-by: Orhun Parmaksız <orhunparmaksiz@gmail.com>

orhun

Some minor things left :)

The fixture config can be simplified.
A small tweak in docs is needed (see my comment)
We need an issue to track the deprecation of config

.github/fixtures/test-topo-order-commits/cliff.toml

website/docs/configuration/git.md

SebClapie · 2025-04-03T09:50:27Z

Some minor things left :)

The fixture config can be simplified.

A small tweak in docs is needed (see my comment)

We need an issue to track the deprecation of config

Hello @orhun
what do you mean about the deprecation of config ?
You want to replace topo_order by topo_order_tags ?

Cyclonit · 2025-04-03T18:39:48Z

Some minor things left :)

The fixture config can be simplified.

A small tweak in docs is needed (see my comment)

We need an issue to track the deprecation of config

Hello @orhun what do you mean about the deprecation of config ? You want to replace topo_order by topo_order_tags ?

The current config schema has grown historically and is in need of a refactor. I attempted to do this back in #541, but couldn't keep up with other changes at the time. The goal of improving the config in general remains though.

One of the requirements for overhauling the config is the ability to soft deprecate old options. Otherwise any change to the config would be an immediate breaking change.

I don't think you need to care about that right now.

orhun

LGTM, thanks!

We'll probably talk about the deprecation of some options during the config revamp, which should be git-cliff v3 eventually.

welcome · 2025-04-05T16:58:47Z

Congrats on merging your first pull request! ⛰️

SebClapie requested a review from orhun as a code owner March 26, 2025 10:27

SebClapie force-pushed the main branch from d1f7578 to 553b70a Compare March 26, 2025 11:07

SebClapie force-pushed the main branch 9 times, most recently from 114673f to 30baa95 Compare March 26, 2025 15:46

SebClapie changed the title ~~feat(git): support sorting commits topological/time (#804)~~ feat(git): disable sorting commits topologically (#804) Mar 26, 2025

orhun reviewed Mar 27, 2025

View reviewed changes

SebClapie force-pushed the main branch 6 times, most recently from 3f7f698 to bd21beb Compare March 27, 2025 16:34

SebClapie force-pushed the main branch from bd21beb to 093b0d9 Compare March 31, 2025 06:32

Cyclonit reviewed Mar 31, 2025

View reviewed changes

.github/fixtures/test-topo-order-commits/cliff.toml Show resolved Hide resolved

SebClapie force-pushed the main branch 2 times, most recently from 9020f27 to 12d4778 Compare March 31, 2025 13:46

SebClapie force-pushed the main branch 3 times, most recently from 641e0f0 to 1dab65b Compare March 31, 2025 14:23

feat(git): disable sorting commits topologically (orhun#804)

430b8f6

SebClapie force-pushed the main branch from 1dab65b to 430b8f6 Compare March 31, 2025 14:38

Cyclonit reviewed Mar 31, 2025

View reviewed changes

config/cliff.toml Outdated Show resolved Hide resolved

orhun reviewed Apr 1, 2025

View reviewed changes

website/docs/configuration/git.md Outdated Show resolved Hide resolved

SebClapie and others added 2 commits April 2, 2025 07:57

Update config/cliff.toml

8f07b6b

Co-authored-by: Orhun Parmaksız <orhunparmaksiz@gmail.com>

Update website/docs/configuration/git.md

13964a7

Co-authored-by: Orhun Parmaksız <orhunparmaksiz@gmail.com>

orhun reviewed Apr 2, 2025

View reviewed changes

.github/fixtures/test-topo-order-commits/cliff.toml Outdated Show resolved Hide resolved

website/docs/configuration/git.md Outdated Show resolved Hide resolved

SebClapie and others added 2 commits April 3, 2025 10:37

Update .github/fixtures/test-topo-order-commits/cliff.toml

cae5ad9

Co-authored-by: Orhun Parmaksız <orhunparmaksiz@gmail.com>

Update website/docs/configuration/git.md

5da5949

Co-authored-by: Orhun Parmaksız <orhunparmaksiz@gmail.com>

orhun reviewed Apr 3, 2025

View reviewed changes

.github/fixtures/test-topo-order-commits/cliff.toml Show resolved Hide resolved

website/docs/configuration/git.md Show resolved Hide resolved

SebClapie added 2 commits April 3, 2025 11:54

Update website documentation

3d8544e

Simplify configuration

25b5c37

SebClapie force-pushed the main branch from 67c770c to 25b5c37 Compare April 3, 2025 09:55

orhun approved these changes Apr 5, 2025

View reviewed changes

orhun merged commit c3e25c3 into orhun:main Apr 5, 2025
73 checks passed

BrewTestBot mentioned this pull request Jun 2, 2025

git-cliff 2.9.0 Homebrew/homebrew-core#225459

Merged

Uh oh!

feat(git): disable sorting commits topologically (#804) #1121

feat(git): disable sorting commits topologically (#804) #1121

Uh oh!

Conversation

SebClapie commented Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

How Has This Been Tested?

Screenshots / Logs (if applicable)

Types of Changes

Checklist:

Uh oh!

welcome bot commented Mar 26, 2025

Uh oh!

codecov-commenter commented Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

orhun left a comment

Choose a reason for hiding this comment

Uh oh!

Cyclonit commented Mar 27, 2025

Uh oh!

SebClapie commented Mar 27, 2025

Uh oh!

orhun commented Mar 28, 2025

Uh oh!

SebClapie commented Mar 31, 2025

Uh oh!

Cyclonit commented Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

SebClapie commented Mar 31, 2025

Uh oh!

Uh oh!

orhun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

orhun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

orhun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

SebClapie commented Apr 3, 2025

Uh oh!

Cyclonit commented Apr 3, 2025

Uh oh!

orhun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

welcome bot commented Apr 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

SebClapie commented Mar 26, 2025 •

edited

Loading

codecov-commenter commented Mar 26, 2025 •

edited

Loading

Cyclonit commented Mar 31, 2025 •

edited

Loading