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

fix: fall back to conventional commit-parser settings for missing keys #496

Merged
merged 4 commits into from
Nov 30, 2018

Conversation

marionebl
Copy link
Contributor

@marionebl marionebl commented Nov 25, 2018

Fixes #341, #399, #445

BREAKING CHANGE
This potentially changes implicit commitlint behaviour users may
have relied on in earlier versions. Instead of falling back to the
builtin commit-parser defaults we now default all keys to
conventional-changelog-angular.

@marionebl marionebl mentioned this pull request Nov 25, 2018
4 tasks

const parsed = parser(message, parserOpts);
const defaultOpts = (await defaultChangelogOpts).parserOpts;
const parsed = parser(message, merge({}, defaultOpts, parserOpts));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the defaultOpts and parserOpts deep objects? If not I can imagine that some users need to be informed when they define the property they have to fully define it. E.g.

const defaultOpts = { some: true, other: { abc: true, def: true } };
const parserOpts = { other: { abc: true } };

merge({}, defaultOpts, parserOpts) === {
    some: true,
    other: { abc: true },
};

I don't think we should use mergeDeep, can imagine that this is intended behaviour?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about this topic so I approve the changes, but looking forward how this will end.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a quick runkit notebook for this and I thing lodash.merge does deep merging (fortunately! :))

https://runkit.com/marionebl/5c00b3d947189a00134a944e

BREAKING CHANGE
This potentially changes implicit commitlint behaviour users may
have relied on in earlier versions. Instead of falling back to the
builtin commit-parser defaults we now default all keys to
conventional-changelog-angular.
@marionebl marionebl force-pushed the fix-use-conventional-settings-always branch from 7509dc5 to d058850 Compare November 30, 2018 04:48
@marionebl marionebl merged commit 831a141 into master Nov 30, 2018
@marionebl marionebl deleted the fix-use-conventional-settings-always branch November 30, 2018 04:55
@prisis
Copy link

prisis commented Dec 21, 2018

When is the release of this fix?

@byCedric
Copy link
Member

@prisis see this comment. You can expect a new release somewhere around the first two weeks of 2019 😄

@escapedcat @marionebl Maybe it might be useful for others to prepare a milestone with a due date, so others can read about the next expected release date?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants