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

Add commitizen to package templates #418

Merged
merged 13 commits into from
Apr 19, 2021
Merged

Add commitizen to package templates #418

merged 13 commits into from
Apr 19, 2021

Conversation

kosanna
Copy link
Contributor

@kosanna kosanna commented Apr 16, 2021

We use semantic release as a primary release mechanism for private packages, and many teams and individuals still struggle from not being able to construct semantic commit names, so adding commit script to assist with on-boarding.

@changeset-bot
Copy link

changeset-bot bot commented Apr 16, 2021

🦋 Changeset detected

Latest commit: 905055c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
skuba Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

template/private-npm-package/_package.json Outdated Show resolved Hide resolved
package.json Outdated
@@ -62,6 +62,7 @@
"babel-plugin-macros": "^3.0.1",
"babel-plugin-module-resolver": "^4.1.0",
"chalk": "^4.1.0",
"commitizen": "^4.2.3",
Copy link
Member

Choose a reason for hiding this comment

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

I'm not keen on bundling commitizen into skuba itself while it has high-severity security vulnerabilities that have gone unpatched for months. If we're sure this is still the best tool for the job (what does it do besides present ~2 prompts to the user?) I'd place it in the template package.json.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, can move it to templates then 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess sec is less of a concern for dev deps on packages

"lint": "skuba lint",
"release": "yarn build && skuba release",
"test": "skuba test --coverage",
"test:watch": "skuba test --watch"
},
"config": {
Copy link
Member

Choose a reason for hiding this comment

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

  1. I believe we can/should do the same to the oss-npm-package template.
  2. Should we add a section to the relevant template READMEs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I was just not sure about the oss, just because you could go either way about it and implement changesets for example

Copy link
Member

Choose a reason for hiding this comment

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

👍 It's definitely easier to get Changesets going in oss, but it should be fine to include this out of the box since the template defaults to using Semantic Release anyway.

Comment on lines 123 to 126

In order to facilitate the alignment with semantic release commit naming convention you can use `yarn commit` command. The script will take you through a range of questions, in the end of which you will have a correct commit name and description for semantic release to pick it up.

However, if you are using _squash and merge_ commits you will need to ensure the convention is preserved upon merging the pull request on Github by ensuring title and description are still populated with necesary details and follow the convention.
Copy link
Member

Choose a reason for hiding this comment

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

Should this go in its own section under Release? Something like:

## Release

...

### Commit messages

Here!

### Releasing latest

...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As you wish, I think it makes sense 👌

@kosanna kosanna marked this pull request as ready for review April 19, 2021 01:35
@kosanna kosanna requested a review from a team as a code owner April 19, 2021 01:35
@72636c 72636c changed the title feat(privateNpm): add commitizen to private template package Add commitizen to package templates Apr 19, 2021
Copy link
Member

@72636c 72636c left a comment

Choose a reason for hiding this comment

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

Thanks for this!

@72636c 72636c enabled auto-merge (squash) April 19, 2021 22:44
@72636c 72636c merged commit 4ecd622 into master Apr 19, 2021
@72636c 72636c deleted the add-commitizen branch April 19, 2021 22:51
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.

2 participants