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

fix: the sort order of commits is unstable #210

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ikedam
Copy link

@ikedam ikedam commented Sep 17, 2022

Use sort.SliceStable instead of sort.Slice to sort commits.

Closes #209

What does this do / why do we need it?

This change uses sort.SliceStable to sort commits: https://pkg.go.dev/sort#SliceStable

Current implementation uses sort.Slice, but it's an unstable sort as described in godoc: https://pkg.go.dev/sort#Slice
This results the order of logs indefinite. Especially the order differs when you change .options.header.pattern and the extracted log set changes. I want the order of logs always same. Please refer #209 for an example.

How this PR fixes the problem?

sort.SliceStable is a stable sort and the original order is preserved for same priority elements (for example, logs with the same scope). "the original order" is expected to be the commit order.

What should your reviewer look out for in this PR?

This change results the log order changed. That means the order of logs in previous releases changes when a developer runs git-chglog with this fix for a new release.
Please consider whether this is acceptable. If not, I'll introduce a new cli option or configuration option.
(Anyway, the log order can change when the implementation of sort.Slice in golang is changed...)

Check lists

  • Test passed
  • Coding style (indentation, etc)

Additional Comments (if any)

There are other lines using sort.Slice. I believe they don't need to be changed to sort.SliceStable as their sort key is expected to be unique (e.g. Title).

Which issue(s) does this PR fix?

fixes #209

@mavogel
Copy link
Member

mavogel commented Jan 22, 2023

@ikedam could you rebase on master as well here? :)

@mavogel mavogel added this to the v0.16.0 milestone Jan 22, 2023
Use `sort.SliceStable` instead of `sort.Slice` to sort commits.

Closes git-chglog#209
@ikedam
Copy link
Author

ikedam commented Jan 24, 2023

Rebased.

@mavogel
Copy link
Member

mavogel commented Jan 24, 2023

same here :) checks triggered

@coveralls
Copy link

Pull Request Test Coverage Report for Build 3995581080

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 76.786%

Totals Coverage Status
Change from base Build 3993620083: 0.0%
Covered Lines: 1849
Relevant Lines: 2408

💛 - Coveralls

@rubenfonseca
Copy link

Hi :) We got hit by this problem too, is there any chance you can merge this?

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.

Commit sotring is unstable
4 participants