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!: use cz-conventional-changelog as default adapter #778

Merged
merged 1 commit into from Sep 25, 2020

Conversation

@felipecrs
Copy link
Contributor

@felipecrs felipecrs commented Sep 14, 2020

  • Remove streamich's git-cz from the documentation.
  • Use cz-conventional-changelog even when the repository is not commitizen-friendly

Fixes #762.

@felipecrs felipecrs marked this pull request as draft Sep 14, 2020
@felipecrs felipecrs force-pushed the felipecrs/new-default-adapter branch from 7c570e6 to 2b26cc6 Sep 14, 2020
@dmwelch
Copy link
Contributor

@dmwelch dmwelch commented Sep 16, 2020

Test errors:

40 passing (8m)
1 pending
2 failing

  1. git-cz
    bootstrap
    when config is not provided
    and the config is not returned from configLoader.load
    tells commitizen to use the git strategy:

    AssertionError: expected false to equal true
    + expected - actual

    -false
    +true

    at Context.it (test/tests/cli.js:54:48)

  2. git-cz
    bootstrap
    when argv is overridden
    uses the overridden argv:
    TypeError: Cannot read property '0' of undefined
    at Context.it (test/tests/cli.js:62:16)

Loading

@felipecrs
Copy link
Contributor Author

@felipecrs felipecrs commented Sep 16, 2020

@dmwelch thanks for pointing, there are many things that I'll have to fix. I'll take a look tonight.

Loading

@felipecrs felipecrs force-pushed the felipecrs/new-default-adapter branch 4 times, most recently from c1fc9bd to 41b5590 Sep 20, 2020
@felipecrs felipecrs marked this pull request as ready for review Sep 20, 2020
@felipecrs felipecrs force-pushed the felipecrs/new-default-adapter branch from 41b5590 to 6e265af Sep 20, 2020
@felipecrs
Copy link
Contributor Author

@felipecrs felipecrs commented Sep 20, 2020

@dmwelch ready for review 👍🏻

Loading

@dmwelch dmwelch merged commit e6b75cb into commitizen:master Sep 25, 2020
8 checks passed
Loading
@felipecrs felipecrs deleted the felipecrs/new-default-adapter branch Sep 25, 2020
@commitizen-bot
Copy link

@commitizen-bot commitizen-bot commented Oct 20, 2020

🎉 This PR is included in version 4.2.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Loading

@felipecrs
Copy link
Contributor Author

@felipecrs felipecrs commented Oct 20, 2020

Ops, it was a breaking change (notice the ! in the feat!:), so a major version bump would be needed. What should we do now? Release a new patch version reverting this commit, and then reapply it ensuring it will produce a new major version?

Perhaps the current semantic-release config does not support the Conventional Commits short ! breaking change identifier.

It was also not included in the changelog.

/cc @dmwelch

Loading

@travi
Copy link
Contributor

@travi travi commented Oct 20, 2020

Release a new patch version reverting this commit, and then reapply it ensuring it will produce a new major version?

that would be my recommendation. please don't unpublish. that just causes more trouble for consumers

Perhaps the current semantic-release config does not support the Conventional Commits short ! breaking change identifier.

since semantic-release uses the angular preset by default, youre right that it does not support this syntax. if you configure it to instead use the conventionalcommits preset, i believe that it should be supported.

i see that you've already opened an issue for switching the default, which i think is at least a worthwhile conversation to have, but currently the angular one is used unless configured specifically.

Loading

@dmwelch
Copy link
Contributor

@dmwelch dmwelch commented Oct 20, 2020

I've manually updated the changelog for v4.2.2 to reflect the inclusion of this commit. I was going to revert the changes now, but I'm not very familiar with the semantic-release package so I'm going to hold off and get up to speed first (unless someone else wants to beat me to the punch...).

Loading

@felipecrs
Copy link
Contributor Author

@felipecrs felipecrs commented Oct 20, 2020

So the plan I suggest is:

  • 1. Revert #778 using fix: as the commit type - this should trigger a new patch -> #792
  • 2. Add the preset conventionalcommits to semantic-release configuration -> #793
  • 3. Wait for the patch release to happen, then reapply this PR -> #794

Please merge in the order.

/cc @dmwelch

Loading

felipecrs added a commit to felipecrs/cz-cli that referenced this issue Oct 22, 2020
…izen#778)"

This reverts commit e6b75cb.

It was supposed to be released as a breaking change.
felipecrs added a commit to felipecrs/cz-cli that referenced this issue Oct 22, 2020
…izen#778)"

This reverts commit e6b75cb.

It was supposed to be released as a breaking change.
@felipecrs
Copy link
Contributor Author

@felipecrs felipecrs commented Dec 15, 2020

@jimthedev @dmwelch in case you guys don't have time (I'm not sure who else is maintaining this project), I can help to maintain this repo.

Loading

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

Successfully merging this pull request may close these issues.

4 participants