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): do not skip breaking changes if configured #114

Merged
merged 2 commits into from
Oct 4, 2022

Conversation

sbmueller
Copy link
Contributor

@sbmueller sbmueller commented Oct 1, 2022

Hi there! Trying to contribute to this project first-time.

Description

Trying to resolve #106 by option to protect breaking changes, even when matched by a skipping commit parser. The option protect_breaking_commits is introduced in the config.
Note that during testing, one caveat was discovered: If a commit is matched by one parser, it can't be matched by any other. That means if a breaking commit was prevented from being skipped, it won't become part of any other group. Maybe it is worth to think about allowing multiple parsers for commits in an hierarchical order?

Motivation and Context

Breaking changes are highly relevant for stakeholders, therefore should not be skipped in a changelog, even when matched by commit parsers.

Fixes #106

How Has This Been Tested?

Extended the test cases by breaking change commits. Added a usual breaking change commit and one that matches a skipped commit parser regex. The latter is not skipped even it matches a commit parser that has skip set to true, if protect_breaking_commits is true.

nightly-aarch64-apple-darwin / rustc 1.66.0-nightly (8ce3204af 2022-09-30)

Backward compatibility is provided.

Screenshots / Output (if appropriate):

See tests in src/changelog.rs

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.

@sbmueller sbmueller requested a review from orhun as a code owner October 1, 2022 17:12
@codecov-commenter
Copy link

codecov-commenter commented Oct 2, 2022

Codecov Report

Base: 64.06% // Head: 64.46% // Increases project coverage by +0.41% 🎉

Coverage data is based on head (12e298b) compared to base (f9d4b88).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #114      +/-   ##
==========================================
+ Coverage   64.06%   64.46%   +0.41%     
==========================================
  Files          14       14              
  Lines         790      799       +9     
==========================================
+ Hits          506      515       +9     
  Misses        284      284              
Flag Coverage Δ
unit-tests 64.46% <100.00%> (+0.41%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
git-cliff-core/src/config.rs 60.00% <ø> (ø)
git-cliff-core/src/commit.rs 94.98% <100.00%> (+0.12%) ⬆️
git-cliff-core/tests/integration_test.rs 100.00% <100.00%> (ø)
git-cliff/src/changelog.rs 71.86% <100.00%> (+0.70%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sbmueller sbmueller force-pushed the dont-skip-breaking-commits branch 2 times, most recently from 91726b8 to f4b6e95 Compare October 3, 2022 07:40
@sbmueller sbmueller changed the title feat(changelog)!: breaking changes are never skipped feat(changelog): breaking changes are never skipped Oct 3, 2022
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.

LGTM! Just a few edits needed, see my comments 🐻

git-cliff-core/src/config.rs Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
git-cliff-core/src/commit.rs Outdated Show resolved Hide resolved
git-cliff-core/src/config.rs Show resolved Hide resolved
@orhun
Copy link
Owner

orhun commented Oct 3, 2022

I added test fixtures for this PR in 4e61f30

@sbmueller sbmueller force-pushed the dont-skip-breaking-commits branch from 4e61f30 to 12e298b Compare October 4, 2022 17:58
@sbmueller
Copy link
Contributor Author

Thanks @orhun for the review! I think I addressed everything now. Feel free to add more comments if unsure about something.

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.

Great stuff, thanks for your contribution! 🐻

@orhun orhun changed the title feat(changelog): breaking changes are never skipped feat(changelog): do not skip breaking changes if configured Oct 4, 2022
@orhun orhun merged commit 1c98995 into orhun:main Oct 4, 2022
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.

Allow special handling for "skipped" breaking changes
3 participants