feat(changelog): implement commit processing order as set of steps

[dslemusp]

Description

Currently each commit in the commit list is processed before splitting. This causes an issue when split_commit is set to true. Commits in the description are not being processed if the commit message contains a skipped type set in the commit_parsers. For instance if the output of a squash and merge start with a chore: and the chore: is set to skip in the commit_parsers the description won't be parsed even if it contains feat: or fix: or any other non skipped field.

// Squash and merge Commit Sample
chore(ci): Update ci runners

fix(ci): a fix --> This is never parsed
chore(ci): another chore --> This is never parsed

Motivation and Context

When working with squash and merge it is useful to have the message of the commit as the overview of the changes and the description as the details of the changes. However it should be possible to skip parsing the message as the description of the commit contains all the info, so trying to avoid repetitions.

How Has This Been Tested?

I have adjusted the current unit test to reflect the scenario that I am describing

Types of Changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
• [ ] Documentation (no code change)
• [x] Refactor (refactoring production code)
• [ ] Other

Checklist:

• [x] My code follows the code style of this project.
• [ ] I have updated the documentation accordingly.
• [x] I have formatted the code with rustfmt.
• [x] I checked the lints with clippy.
• [x] I have added tests to cover my changes.
• [x] All new and existing tests passed.

[welcome[bot]]

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

[codecov-commenter]

Codecov Report

:x: Patch coverage is 82.89474% with 13 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 44.33%. Comparing base (85cc05d) to head (7aa35bd). :warning: Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
git-cliff-core/src/changelog.rs	82.44%	13 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1201      +/-   ##
==========================================
+ Coverage   43.46%   44.33%   +0.87%     
==========================================
  Files          22       22              
  Lines        1972     2017      +45     
==========================================
+ Hits          857      894      +37     
- Misses       1115     1123       +8     

Flag	Coverage Δ
unit-tests	44.33% <82.90%> (+0.87%)	:arrow_up:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[orhun]

This was changed in #556 to fix another issue and was reported again in #1093 - can you maybe take a look and double check this?

[dslemusp]

Yeah actually it seems to me that the initial behaviour and order is what I would expect to have. As I said, right now the commits are treated as one during preprocessing even if the split_commit flag is enabled. This results on description being skipped because the commit message is ignored in the preprocessor.

[orhun]

Thanks for checking. I'd lean towards making this configurable instead of changing the behavior again. We could maybe add a new processing_order option like follows:

[git]
processing_order = ["preprocess", "split", "proces", "postprocess", "etc"]

So that you can configure the behavior. What do you think?

[dslemusp]

I like it!

[dslemusp]

How many processing steps do we have?. I see split, preprocess, commit_parsing, link_parsing, into_conventional

[ognis1205]

@dslemusp @orhun

I assume the current step names like preprocess, process, and postprocess are just temporary placeholders for now, since this is still in a draft stage. That said, if we move forward with making the order configurable, I think it’d be good to avoid names like those — they imply a fixed sequence, which might be misleading if the steps can be reordered. It’d be great if we could find names that reflect the actual behavior more clearly.

[dslemusp]

I agree. I would suggest we use similar (if not same) names as they are defined in the cliff.toml (e.g. commit_preprocessor instead of preprocess ). I am working on updating the PR but it might take some time!. Feel free to think about the naming in the mean time ;P

[orhun]

I would suggest we use similar (if not same) names as they are defined in the cliff.toml

Yes, very good point. I'd like that 👍🏼

I am working on updating the PR but it might take some time!

Great, thank you! Let me know if you need any help/guidance in the meantime. I think we should also update the title of this PR accordingly :D

[dslemusp]

Ok I found some time today I came up with this. Maybe there is still some cleanup to do as this makes https://github.com/orhun/git-cliff/blob/c156fc5edb405737b62a802ed9b1e91f9789ef9b/git-cliff-core/src/commit.rs#L208 not needed.

[dslemusp]

@orhun Any follow up on this?. I would like to have this functionality merged

[dslemusp]

Another important point: since there are already issues arising from the interactions of repository and directory-related configs, I think we should be very careful about introducing new functionality into the config. Both the necessity and the exact specification should be carefully evaluated before adding more to this area.

I tend to agree, but in this case the idea is to keep feature parity and backwards comparability in case the processing_steps field is not present in the cliff.toml When I have time I refactor the tests to leave them as they were on the base and add new ones for the feature. This to also check in a way that there are no regressions.

[orhun]

Sorry for the delay on this... but most things look good already. Just need to find some time to do proper review.

Pretty excited for this one

Btw can you fix the conflicts with main?

[orhun]

going to include this in 2.12.0