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

Change the behavior of skip_tags not to drop commits but to include them in the next tag #36

Closed
kenji-miyake opened this issue Nov 28, 2021 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@kenji-miyake
Copy link
Contributor

kenji-miyake commented Nov 28, 2021

Is your feature request related to a problem? Please describe.

While testing this tool, I felt the skip_tags option behaves unnaturally.
The current behavior seems completely dropping commits from the changelog, but I believe that if a certain tag is skipped, it should be included in the next tag.

So for example, supposing the commits and tags are like the following and that skip_tags is v0.2.0-beta.1,

[v0.2.0] feat: add B
[v0.2.0-beta.1] fix: fix A
[v0.1.0] feat: add A

The current behavior is:

[v0.2.0]

## Feature

- Add B

[v0.1.0]

## Feature

- Add A

However, it should be:

[v0.2.0]

## Feature

- Add B

## Fix

- Fix A

[v0.1.0]

## Feature

- Add A

Describe the solution you'd like

Changing the behavior of skip_tags can solve the problem, I believe.

This is an example of the implementation.
kenji-miyake@89d0bf4

How to test:

$ git clone git@github.com:kenji-miyake/git-cliff.git -b main
$ cd git-cliff

$ # Before
$ cargo run | grep -A 20 "0.1.0-beta.2"
   Compiling git-cliff v0.4.2 (/tmp/git-cliff/git-cliff)
    Finished dev [unoptimized + debuginfo] target(s) in 2.20s
     Running `target/debug/git-cliff`
 WARN  git_cliff > "cliff.toml" is not found, using the default configuration.
## [0.1.0-beta.2] - 2021-06-14

### Bug Fixes

- Install musl-tools for musl builds

### Miscellaneous Tasks

- Update config

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

$ # After
$ git checkout fix-skip-tags-behavior
$ cargo run | grep -A 20 "0.1.0-beta.2"
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/git-cliff`
 WARN  git_cliff > "cliff.toml" is not found, using the default configuration.
## [0.1.0-beta.2] - 2021-06-14

### Bug Fixes

- Add timestamp field to Release structs
- Update tests about recent changes
- Update fields
- Require -u or -l flag for prepending changelog
- Write to the given file for prepend operation
- Install zlib dependency
- Install musl-tools for musl builds

### Documentation

- Add doc comment to GroupParser
- Add comments
- Add FUNDING.yml
- Add comments to struct fields
- Add doc comment to generate method
- Add comments to main
- Mention docker

Describe alternatives you've considered

Seeing your use case, it seems you'd like to drop commits before v0.1.0-beta.1, which has a lot of commits.
Therefore, I guess it's nice to add another option like skip_before or so.

Additional context

Since we can drop commits by setting skip = true in commit_parsers, I guess there is no problem with changing like this.

@kenji-miyake kenji-miyake added the enhancement New feature or request label Nov 28, 2021
@kenji-miyake
Copy link
Contributor Author

kenji-miyake commented Nov 28, 2021

@orhun Hi, thank you for developing this awesome project!
Could you take a look at this, please? 🙏

@orhun
Copy link
Owner

orhun commented Nov 29, 2021

Hello!

Thanks for the detailed report and example implementation 🐻

While testing this tool, I felt the skip_tags option behaves unnaturally. The current behavior seems completely dropping commits from the changelog, but I believe that if a certain tag is skipped, it should be included in the next tag.

That's completely fair!

Changing the behavior of skip_tags can solve the problem, I believe.

Seeing your use case, it seems you'd like to drop commits before v0.1.0-beta.1, which has a lot of commits. Therefore, I guess it's nice to add another option like skip_before or so.

I think it would be better to add a new option to the configuration file such as ignore_tags or skip_before. Or even better:

skip_tags = { regex = "v0.2.0-beta.1", drop = true }

So, if drop is set to true it will behave like you demonstrated and vice versa.

This is an example of the implementation. kenji-miyake@89d0bf4

I liked this approach, it can be shaped into something that would allow both use cases for providing high extensibility. Do you want to finalize this and submit a PR or should I take it from here? What do you say?

Since we can drop commits by setting skip = true in commit_parsers, I guess there is no problem with changing like this.

That's correct I'd say.

@kenji-miyake
Copy link
Contributor Author

kenji-miyake commented Nov 29, 2021

@orhun Thank you for the quick reply!

skip_tags = { regex = "v0.2.0-beta.1", drop = true } sounds good to me.
Can it be multiple as well as commit_parsers? I think it has high extensibility.

  • Drop only before "v0.1.0-beta.1"
  • Skip beta releases
skip_tags = [
    { regex = "v0.1.0-beta.1", drop = true },
    { regex = "v.*-beta.*", drop = false },
]
  • Drop before "v0.3.*"
  • Skip beta releases
skip_tags = [
    { regex = "v0\.[0-3]\..*", drop = true },
    { regex = "v.*-beta.*", drop = false },
]

Or considering the use cases, I feel like adding a new option is enough. 🤔
Do you come up with some use cases that needs skip_tags = { regex = "v0.2.0-beta.1", drop = true }?

skip_tags = "v0.1.0-beta.1"
ignore_tags = "v.*-beta.*"
skip_tags = "v0\.[0-3]\..*"
ignore_tags = "v.*-beta.*"

Also then, to keep the naming consistent, ignore_tags would be better?

  • skip
    • Drop commits from the changelog. (commit_parsers)
    • Drop tags and their commits. (skip_tags)
  • ignore
    • Don't take tags into consideration. (ignore_tags)

I feel adding drop may cause confusion because there is already skip as a similar meaning. What do you think about it?

I liked this approach, it can be shaped into something that would allow both use cases for providing high extensibility. Do you want to finalize this and submit a PR or should I take it from here? What do you say?

If possible, I'd like to send a PR (after discussing the approach here) and get reviews from you, it that okay for you?
Or if you (or somebody else who viewed this issue) want to implement this quickly by yourself, it's fine.

@orhun
Copy link
Owner

orhun commented Nov 30, 2021

I feel adding drop may cause confusion because there is already skip as a similar meaning. What do you think about it?

Ah, you are right. Having drop and skip might confuse people 😕 Also we need to take the backwards compatibility into consideration: changing the behavior or config field of skip_tags will break a lot of user's configs.

So adding a new field called ignore_tags is the finest solution here I think.

skip_tags = "v0.1.0-beta.1"
ignore_tags = "v.*-beta.*"
skip_tags = "v0\.[0-3]\..*"
ignore_tags = "v.*-beta.*"

LGTM!

If possible, I'd like to send a PR (after discussing the approach here) and get reviews from you, it that okay for you? Or if you (or somebody else who viewed this issue) want to implement this quickly by yourself, it's fine.

Feel free to submit a PR 🐻 I'll review it as quickly as possible 👍🏼

orhun pushed a commit that referenced this issue Dec 7, 2021
* feat: add ignore_tags option

Resolves (#36)

Signed-off-by: Kenji Miyake <kenji.miyake@tier4.jp>

* test: add a test using GitHub Actions

Signed-off-by: Kenji Miyake <kenji.miyake@tier4.jp>

* docs: fix style

Signed-off-by: Kenji Miyake <kenji.miyake@tier4.jp>

* chore: add ignore_tags to example config files

Signed-off-by: Kenji Miyake <kenji.miyake@tier4.jp>

* style: refactor test-fixtures workflow

Signed-off-by: Kenji Miyake <kenji.miyake@tier4.jp>
@orhun
Copy link
Owner

orhun commented Dec 11, 2021

This is implemented now. Closing out!

@orhun orhun closed this as completed Dec 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants