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, config)!: replace --topo-order by --date-order #58

Merged
merged 19 commits into from
Feb 12, 2022
Merged

feat(changelog, config)!: replace --topo-order by --date-order #58

merged 19 commits into from
Feb 12, 2022

Conversation

kenji-miyake
Copy link
Contributor

@kenji-miyake kenji-miyake commented Feb 5, 2022

Description

Resolves #54

As explained in #54 (comment), I replaced --topo-order by --date-order, which means the default value will be date_order=false (topo_order=true).

Motivation and Context

since topo-order is already default for git, maybe it should be default for git-cliff
in that case, --topo-order flag is unnecessary and should be renamed to --date-order

How Has This Been Tested?

The test script in #54 (comment) worked correctly.

Screenshots / Output (if appropriate):

output
$ ./test.sh
Initialized empty Git repository in /tmp/test/.git/
[main (root-commit) 02b906e] Initial commit
[main 5b8ba4b] feat: v0.1.0
Note: switching to 'v0.1.0'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 5b8ba4b feat: v0.1.0
[detached HEAD f7c021e] feat: v0.2.0
Previous HEAD position was f7c021e feat: v0.2.0
HEAD is now at 5b8ba4b feat: v0.1.0
[detached HEAD 3b5dfe3] fix: v0.1.1
Previous HEAD position was 3b5dfe3 fix: v0.1.1
HEAD is now at f7c021e feat: v0.2.0
[detached HEAD 0c8228c] feat: v0.3.0
Previous HEAD position was 0c8228c feat: v0.3.0
HEAD is now at 5b8ba4b feat: v0.1.0
------
v0.1.0
 WARN  git_cliff > "cliff.toml" is not found, using the default configuration.
# Changelog
All notable changes to this project will be documented in this file.

## [0.1.0] - 2022-02-05

### Features

- V0.1.0

<!-- generated by git-cliff -->
------
Previous HEAD position was 5b8ba4b feat: v0.1.0
HEAD is now at 3b5dfe3 fix: v0.1.1
------
v0.1.1
 WARN  git_cliff > "cliff.toml" is not found, using the default configuration.
# Changelog
All notable changes to this project will be documented in this file.

## [0.1.1] - 2022-02-05

### Bug Fixes

- V0.1.1

<!-- generated by git-cliff -->
------
Previous HEAD position was 3b5dfe3 fix: v0.1.1
HEAD is now at f7c021e feat: v0.2.0
------
v0.2.0
 WARN  git_cliff > "cliff.toml" is not found, using the default configuration.
# Changelog
All notable changes to this project will be documented in this file.

## [0.2.0] - 2022-02-05

### Features

- V0.2.0

<!-- generated by git-cliff -->
------
Previous HEAD position was f7c021e feat: v0.2.0
HEAD is now at 0c8228c feat: v0.3.0
------
v0.3.0
 WARN  git_cliff > "cliff.toml" is not found, using the default configuration.
# Changelog
All notable changes to this project will be documented in this file.

## [0.3.0] - 2022-02-05

### Features

- V0.3.0

<!-- generated by git-cliff -->
------

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.

@kenji-miyake
Copy link
Contributor Author

@orhun I've submitted a draft PR to check tests.

@codecov-commenter
Copy link

codecov-commenter commented Feb 5, 2022

Codecov Report

Merging #58 (543d2f8) into main (0293b28) will increase coverage by 0.13%.
The diff coverage is 11.12%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #58      +/-   ##
==========================================
+ Coverage   40.90%   41.02%   +0.13%     
==========================================
  Files          13       13              
  Lines         824      824              
  Branches      215      216       +1     
==========================================
+ Hits          337      338       +1     
+ Misses        378      375       -3     
- Partials      109      111       +2     
Impacted Files Coverage Δ
git-cliff-core/src/config.rs 24.45% <0.00%> (ø)
git-cliff-core/src/repo.rs 28.13% <0.00%> (+1.05%) ⬆️
git-cliff/src/args.rs 0.00% <0.00%> (ø)
git-cliff/src/lib.rs 0.72% <0.00%> (ø)
git-cliff/src/changelog.rs 72.98% <100.00%> (ø)

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 0293b28...543d2f8. Read the comment docs.

@kenji-miyake
Copy link
Contributor Author

It seems there were no tests that compare topo_order=true/false.
So I'll just add new fixtures tests for this.

@kenji-miyake
Copy link
Contributor Author

kenji-miyake commented Feb 5, 2022

@orhun Are there any reasons for changing cargo run to Docker here? 🤔
92a54d6

I guess orhunp/git-cliff:latest can't test the latest pull-request code, so can I replace it back with cargo run?

@orhun
Copy link
Owner

orhun commented Feb 6, 2022

@orhun Are there any reasons for changing cargo run to Docker here? thinking 92a54d6

Yeah, I made that change so that workflow will complete faster. Compiling takes time...

I guess orhunp/git-cliff:latest can't test the latest pull-request code, so can I replace it back with cargo run?

Yup, now I see that we can revert it. I wonder if there is a way to make this run faster though. (totally out of scope of this PR)

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.

I have nitpicks here and there but overall it looks good 👍🏼 Good job with the fixtures.

.github/actions/run-fixtures-test/action.yml Outdated Show resolved Hide resolved
.github/actions/run-fixtures-test/action.yml Outdated Show resolved Hide resolved
.github/actions/run-fixtures-test/action.yml Outdated Show resolved Hide resolved
.github/fixtures/test-date-order-arg/cliff.toml Outdated Show resolved Hide resolved
.github/actions/run-fixtures-test/action.yml Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Kenji Miyake added 19 commits February 9, 2022 01:13
Signed-off-by: Kenji Miyake <[email protected]>
Signed-off-by: Kenji Miyake <[email protected]>
Signed-off-by: Kenji Miyake <[email protected]>
Signed-off-by: Kenji Miyake <[email protected]>
Signed-off-by: Kenji Miyake <[email protected]>
Signed-off-by: Kenji Miyake <[email protected]>
Signed-off-by: Kenji Miyake <[email protected]>
Signed-off-by: Kenji Miyake <[email protected]>
Signed-off-by: Kenji Miyake <[email protected]>
Signed-off-by: Kenji Miyake <[email protected]>
Signed-off-by: Kenji Miyake <[email protected]>
This reverts commit 8028815.
This reverts commit 3d334cc.
Signed-off-by: Kenji Miyake <[email protected]>
@orhun orhun merged commit a3980f4 into orhun:main Feb 12, 2022
@kenji-miyake kenji-miyake deleted the replace-topo-order-by-dete-order branch February 12, 2022 17:08
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.

The --current option involves the release notes of other tags
3 participants