refactor(config)!: revamp configuration #559

Cyclonit · 2024-03-16T16:57:06Z

Description

Implementation of #541. This PR aims to clean up the configuration of git-cliff in several ways.

Improve names for configuration options to improve clarity.
Improve descriptions and documentation for options.
Restructure configuration options to improve clarity.

Note: Every option was refactored in a separate commit to allow for detailed reviews and targeted changes.

Motivation and Context

Currently the configuration is fairly convoluted and difficult to understand. For example, several options like git.skip_tags and git.ignore_tags have names that are not descriptive and can easily be confused.

How Has This Been Tested?

All existing tests are successful for every commit in this PR. This PR does not contain any functional changes.

Screenshots / Logs (if applicable)

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 formatted the code with rustfmt.
I checked the lints with clippy.
I have added tests to cover my changes.
All new and existing tests passed.

welcome · 2024-03-16T16:57:09Z

Thanks for opening this pull request! Please check out our contributing guidelines! ⛰️

codecov-commenter · 2024-03-16T17:01:19Z

Codecov Report

Attention: Patch coverage is 23.44498% with 160 lines in your changes are missing coverage. Please review.

Project coverage is 39.04%. Comparing base (d53b905) to head (f69dfe4).

Files	Patch %	Lines
git-cliff/src/config.rs	0.00%	44 Missing ⚠️
git-cliff/src/lib.rs	0.00%	37 Missing ⚠️
git-cliff-core/src/config/embed.rs	0.00%	26 Missing ⚠️
git-cliff-core/src/changelog.rs	51.73%	14 Missing ⚠️
git-cliff-core/src/config/migrate.rs	0.00%	12 Missing ⚠️
git-cliff-core/src/config/models_v2.rs	56.53%	10 Missing ⚠️
git-cliff-core/src/config/parsing.rs	43.75%	9 Missing ⚠️
git-cliff/src/main.rs	0.00%	7 Missing ⚠️
git-cliff-core/src/commit.rs	92.86%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #559      +/-   ##
==========================================
- Coverage   41.52%   39.04%   -2.47%     
==========================================
  Files          15       18       +3     
  Lines        1072     1145      +73     
==========================================
+ Hits          445      447       +2     
- Misses        627      698      +71

Flag	Coverage Δ
unit-tests	`39.04% <23.45%> (-2.47%)`	⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Added a subcommand migrate-config to migrate from the old to the new configuration format.

Cyclonit · 2024-04-15T14:43:52Z

Hi @orhun,
I consider this PR to be ready for review. All fixtures are green. The only thing remaining is fixing the linting messages that are caused by the deprecated v1 config classes.

orhun · 2024-05-11T18:01:45Z

Inviting @MarcoIeni @alerque @kenji-miyake @tranzystorekk for thoughts and review 🤗

MarcoIeni · 2024-05-11T19:32:36Z

.github/fixtures/new-fixture-template/cliff.toml

-# https://keats.github.io/tera/docs/#introduction
-body = """
+# A Tera template to be rendered for each release in the changelog (see https://keats.github.io/tera/docs/#introduction).
+body_template = """


can't we make header a template too and remove _template from all the changelog parts?
Probably this suggestion would have been better when discussing the issue, but I didn't pay enough attention, so sorry about that!

MarcoIeni · 2024-05-11T19:34:08Z

.github/fixtures/new-fixture-template/cliff.toml

-# remove the leading and trailing whitespace from the templates
-trim = true
+# Whether to remove leading and trailing whitespaces from all lines of the changelog's body.
+trim_body_whitespace = true


doesn't this also trim new lines?
Maybe trim_body is enough as a name. 🤔

MarcoIeni · 2024-05-11T19:38:17Z

.github/fixtures/test-github-integration/cliff.toml

+[commit]
+# Whether to parse commits according to the conventional commits specification.
+# Sets the commits' `group` (= `type`), `scope`, `message` (= `description`), `body`, `breaking`, `breaking_description` and `footers`.
+parse_conventional_commits = false


I don't think parse_ adds much more clarity here, so imo we could also avoid adding it to spare this breaking change.
But this comes to personal taste :)

+1 for the name, but this option feels weirdly interlinked with exclude_unconventional_commits, a single option like conventional_commits=<strict|enabled|disabled> where strict means "also exclude unconventional" seems more intuitive

MarcoIeni · 2024-05-11T19:39:40Z

.github/fixtures/test-github-integration/cliff.toml

+# Sets the commits' `group` (= `type`), `scope`, `message` (= `description`), `body`, `breaking`, `breaking_description` and `footers`.
+parse_conventional_commits = false
+# Whether to exclude commits that do not match the conventional commits specification from the changelog.
+exclude_unconventional_commits = true


I like exclude more than filter. It's more clear 👍
Since we are already in the [commit] section, should we drop _commits? Same for parse_conventional_commits

MarcoIeni · 2024-05-11T19:41:59Z

.github/fixtures/test-ignore-tags/cliff.toml

+
+[commit]
+# Regex to select git tags that should be excluded from the changelog.
+exclude_tags_pattern = "v0.1.0-beta.1"


Should we pick one between exclude_ and skip_?

MarcoIeni · 2024-05-11T19:44:28Z

.github/fixtures/test-keep-a-changelog-links-current-arg/cliff.toml

+skip_tags_pattern = ""
+# Whether to order releases chronologically or topologically.
+# Must be either `time` or `topology`.
+order_by = "time"


should we merge order_by and sort_order?
E.g. introduce a new sort config that can be one of the following:

newest

oldest

topology

AFAIU sort_order describes sorting in commit groups, while order_by is for releases, so we can't merge unrelated options

Cyclonit requested a review from orhun as a code owner March 16, 2024 16:57

Cyclonit force-pushed the feature/revamp_configuration branch 2 times, most recently from 44f1335 to f677ceb Compare March 16, 2024 17:00

Cyclonit force-pushed the feature/revamp_configuration branch 5 times, most recently from 14d2f11 to 25121ec Compare March 19, 2024 06:23

Cyclonit marked this pull request as draft March 19, 2024 06:26

Cyclonit force-pushed the feature/revamp_configuration branch 7 times, most recently from d0e907b to bfb7fdf Compare March 24, 2024 11:32

orhun mentioned this pull request Mar 24, 2024

Generate multiple changelogs in different formats #572

Open

1 task

Cyclonit force-pushed the feature/revamp_configuration branch 7 times, most recently from 339b9bd to 5ce1b9e Compare April 14, 2024 06:11

Cyclonit force-pushed the feature/revamp_configuration branch from 5ce1b9e to 1548930 Compare April 14, 2024 08:28

Cyclonit added 3 commits April 14, 2024 17:23

feature(config): introduce new configuration schema

2193936

feature(config): add migrate-config subcommand

a3f2f8b

Added a subcommand migrate-config to migrate from the old to the new configuration format.

feature(config): add support for both configuration versions

8ead718

doc(config): add documentation for configuration revamp

02bf5bb

Cyclonit force-pushed the feature/revamp_configuration branch 12 times, most recently from fe6d2bd to a483d71 Compare April 15, 2024 07:18

Cyclonit added 2 commits April 15, 2024 10:00

feature(config): improve support for backwards compatability

d7679c6

test(config): add tests for backwards compatibility

f69dfe4

Cyclonit force-pushed the feature/revamp_configuration branch from a483d71 to f69dfe4 Compare April 15, 2024 08:00

Cyclonit marked this pull request as ready for review April 15, 2024 10:57

MarcoIeni reviewed May 11, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(config)!: revamp configuration #559

refactor(config)!: revamp configuration #559

Cyclonit commented Mar 16, 2024 •

edited

welcome bot commented Mar 16, 2024

codecov-commenter commented Mar 16, 2024 •

edited

Cyclonit commented Apr 15, 2024

orhun commented May 11, 2024

MarcoIeni May 11, 2024 •

edited

MarcoIeni May 11, 2024

MarcoIeni May 11, 2024

tranzystorekk May 11, 2024

MarcoIeni May 11, 2024

MarcoIeni May 11, 2024

MarcoIeni May 11, 2024 •

edited

tranzystorekk May 11, 2024

refactor(config)!: revamp configuration #559

Are you sure you want to change the base?

refactor(config)!: revamp configuration #559

Conversation

Cyclonit commented Mar 16, 2024 • edited

Description

Motivation and Context

How Has This Been Tested?

Screenshots / Logs (if applicable)

Types of Changes

Checklist:

welcome bot commented Mar 16, 2024

codecov-commenter commented Mar 16, 2024 • edited

Codecov Report

Cyclonit commented Apr 15, 2024

orhun commented May 11, 2024

MarcoIeni May 11, 2024 • edited

Choose a reason for hiding this comment

MarcoIeni May 11, 2024

Choose a reason for hiding this comment

MarcoIeni May 11, 2024

Choose a reason for hiding this comment

tranzystorekk May 11, 2024

Choose a reason for hiding this comment

MarcoIeni May 11, 2024

Choose a reason for hiding this comment

MarcoIeni May 11, 2024

Choose a reason for hiding this comment

MarcoIeni May 11, 2024 • edited

Choose a reason for hiding this comment

tranzystorekk May 11, 2024

Choose a reason for hiding this comment

Cyclonit commented Mar 16, 2024 •

edited

codecov-commenter commented Mar 16, 2024 •

edited

MarcoIeni May 11, 2024 •

edited

MarcoIeni May 11, 2024 •

edited