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

Allow overwriting whatBump of presets #128

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Weissger
Copy link

The implementation of conventional-recommended-bump allows to pass a whatBump function which isn't the result of preset loading.

I added a simple passthrough from args to the conventionalRecommendedBump call to allow easily overwriting the preset defined whatBump.

Why was this added

Since the release of following changes in v11.2.3 I got an issue with my use-case for using the library.

I'm using custom notes / footers in my commit messages using the tool. This should follow the conventional commit spec

footers other than BREAKING CHANGE: may be provided and follow a convention similar to git trailer format.

Unfortunately the basic whatBump function interprets every single commit containing a footer as BREAKING CHANGE.

Generally the logic of conventional-changelog-conventionalcommits should be updated, but allowing for overwriting the function of a preset gives a nice flexibility for all presets and customized applications of the library.

@TimothyJones
Copy link
Member

Thanks for the thorough description! Could you add the feature to the readme, and also add a test?

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7ebdacd) 94.11% compared to head (a4f9573) 94.11%.

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

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #128   +/-   ##
=======================================
  Coverage   94.11%   94.11%           
=======================================
  Files          26       26           
  Lines         493      493           
=======================================
  Hits          464      464           
  Misses         29       29           

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

@Weissger
Copy link
Author

Weissger commented Feb 20, 2024

Thanks for the thorough description! Could you add the feature to the readme, and also add a test?

Sure. Can do in the coming days / weeks.

Alternatively (or additionally) to this change I would prefer if commit-and-tag-version would allow for defining differing ParseOpts/WriterOpts for the bump and changelog lifecycle as users might want to handle them slightly differently. This would already resolve my current issue without the need for an overwrite of the whatBump function.

Otherwise v11.2.3 removed a hidden feature I was relying on 😄

@Weissger
Copy link
Author

It also looks like conventional-changelog-conventionalcommits unfortunately has another hardcoded part in the writer transforming the tiles of all existing footers into 'BREAKING CHANGES'.

@TimothyJones
Copy link
Member

Apologies for breaking behaviour without realising!

tbh, I’m not wild about the way this package exposes the internals of the conventional changelog packages- it makes it hard to reason about breaking changes. I think it would be a pretty substantial change to decouple this, though

Since that’s probably a substantial refactor, I think allowing separate parseOpts / writerOpts is a reasonable concession- I’d happily accept a PR that does that if you have the time to raise one.

@TimothyJones
Copy link
Member

Heya! Did you get a chance to add this to the documentation / tests?

@Weissger
Copy link
Author

Hey! Unfortunately life happened and I didn't find time until now. I'll try to follow up as soon as possible.

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.

3 participants