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

Put newest commit on top in CHANGELOG.md #15

Merged
merged 3 commits into from
Sep 28, 2021

Conversation

kondanta
Copy link
Contributor

Description

Change the way git-cliff sorts commit messages on CHANGELOG.md.

Motivation and Context

Normally, I would expect the latest change on the top in CHANGELOG. However, currently, git-cliff puts the oldest commit on the top of the sections(#Features, Fixes and such).

How Has This Been Tested?

I tested this using both a mock repo and git-cliff's own repo.

Screenshots / Output (if appropriate):

- Support parsing the missing scopes with `default_scope` (#8)
- Support generating a changelog scoped to a directory (#11)

With this update, it will look like this:

- Support generating a changelog scoped to a directory (#11)
- Support parsing the missing scopes with `default_scope` (#8)

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 added tests to cover my changes.
  • All new and existing tests passed.

Previously, cliff was sorting the commits by oldest first.
Like:

```
 - Support parsing the missing scopes with `default_scope` (orhun#8)
 - Support generating a changelog scoped to a directory (orhun#11)
```

As the PR numbers indicate, the first bullet point is definitely
older than the latter.

With this update, it will look like this:
```
 - Support generating a changelog scoped to a directory (orhun#11)
 - Support parsing the missing scopes with `default_scope` (orhun#8)
```

Signed-off-by: Taylan Dogan <git@taylandogan.info>
@kondanta kondanta requested a review from orhun as a code owner September 19, 2021 22:27
@codecov-commenter
Copy link

codecov-commenter commented Sep 19, 2021

Codecov Report

Merging #15 (c62ca03) into main (ba3f1ca) will increase coverage by 0.06%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #15      +/-   ##
==========================================
+ Coverage   41.87%   41.92%   +0.06%     
==========================================
  Files          13       13              
  Lines         633      637       +4     
  Branches      168      169       +1     
==========================================
+ Hits          265      267       +2     
  Misses        276      276              
- Partials       92       94       +2     
Impacted Files Coverage Δ
git-cliff/src/args.rs 0.00% <0.00%> (ø)
git-cliff/src/lib.rs 0.99% <0.00%> (-0.01%) ⬇️
git-cliff-core/src/repo.rs 34.53% <0.00%> (+2.39%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba3f1ca...c62ca03. Read the comment docs.

@orhun
Copy link
Owner

orhun commented Sep 21, 2021

I think it'd be better to have a flag to make the sorting optional :)

@kondanta
Copy link
Contributor Author

kondanta commented Sep 25, 2021

Alright, sounds good to me. Do you have anything in mind?

The first thing comes to my mind is something similar to this.

pub struct Opt {
...
       pub sort: Option<Sort>,
}

#[derive(StructOpt, Debug)]
pub enum Sort {
       Newest,
       Oldest,
}

WDYT?

@orhun
Copy link
Owner

orhun commented Sep 27, 2021

WDYT?

Looks good to me. And obviously --sort oldest should be the default.

I don't expect any other sorting types will be added
so the logic consist of checking whether it is
`newest` or not.

One could argue with why wouldn't I make this a boolean.
My answer would be, in my opinion, it lose its meaning
because this is not something we want to enable or disable
but something that we want to decide which pattern
we want to use. So it is more like a semantic choice.

Signed-off-by: Taylan Dogan <git@taylandogan.info>
@kondanta
Copy link
Contributor Author

WDYT?

Looks good to me. And obviously --sort oldest should be the default.

Well, I decided to use a simple string. My reasoning is:

  • I don't expect there will be more sorting logic
  • I don't want to implement FromStr trait for Sort enum.

So what do you think about these new changes? @orhun

@orhun
Copy link
Owner

orhun commented Sep 28, 2021

Looks good to me! Only one thing, I moved the explanation of this flag to README.md in a0635ee

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.

🥂

@orhun orhun merged commit 2950a41 into orhun:main Sep 28, 2021
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.

3 participants