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

chore: setup conventional commits with Lerna #8415

Closed
wants to merge 1 commit into from

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Aug 3, 2018

Description

This PR tries to enabled conventional commits with Lerna which would allows us to automatically regenerate CHANGELOG.md files every time we publish packages to npm. This might also simplify creating Gutenberg changelog for release similar to what we did in #8278.

It's based on Lerna repository configuration and the following article: https://michaljanaszek.com/blog/lerna-conventional-commits.

It is blocked by a known bug in Lerna itself: lerna/lerna#1532:

With a lerna setup with version: independent and conventionalCommits: true for the publish command in lerna.json: when publishing a new release with lerna publish lerna takes all previous commits into account and adds all commits again into CHANGELOG.md for this release and also raises the version respecting all previous commits.

TODO

  • Update Repository management's section about merging PRs to include details how to properly word title and description based on Angular (unless we want to create our own convention but that requires some coding.

How has this been tested?

Can you actually test without publishing? I guess, NO.

@gziolo gziolo added the [Status] Blocked Used to indicate that a current effort isn't able to move forward label Aug 3, 2018
@gziolo gziolo self-assigned this Aug 3, 2018
@gziolo gziolo requested review from youknowriad, ntwb and aduth August 3, 2018 05:27
@gziolo gziolo added npm Packages Related to npm packages [Type] Developer Documentation Documentation for developers labels Aug 3, 2018
@gziolo gziolo requested a review from mtias August 3, 2018 05:35
@gziolo
Copy link
Member Author

gziolo commented Aug 3, 2018

@karmatosed @mtias @pento - are we okey with adopting Angular conventions to use when merging PRs?

https://github.com/conventional-changelog/conventional-changelog/tree/master/packages/conventional-changelog-angular

Examples:

Appears under "Features" header, pencil subheader:

feat(pencil): add 'graphiteWidth' option

Appears under "Bug Fixes" header, graphite subheader, with a link to issue #28:

fix(graphite): stop graphite breaking when width < 0.1

Closes #28

Appears under "Performance Improvements" header, and under "Breaking Changes" with the breaking change explanation:

perf(pencil): remove graphiteWidth option

BREAKING CHANGE: The graphiteWidth option has been removed. The default graphite width of 10mm is always used for performance reason.

The following commit and commit 667ecc1 do not appear in the changelog if they are under the same release. If not, the revert commit appears under the "Reverts" header.

revert: feat(pencil): add 'graphiteWidth' option

This reverts commit 667ecc1654a317a13331b17617d973392f415f02.

@gziolo gziolo requested review from karmatosed and pento August 3, 2018 05:37
@pento
Copy link
Member

pento commented Aug 3, 2018

I'm really not excited about having this kind of strict format, without the ability to enforce it, and the ability to provide templates.

@ntwb
Copy link
Member

ntwb commented Aug 3, 2018

Thinking about the current WordPress' commit message format:

https://make.wordpress.org/core/handbook/best-practices/commit-messages/

And ideally having this continue once Gutenberg is merged into core rather than the Angular preset, the jQuery preset is very similar to the current WordPress format:

https://github.com/conventional-changelog/conventional-changelog/tree/master/packages/conventional-changelog-jquery

e.g:

Component: Short Description

Optional Long Description

Fixes #xxx
Closes gh-yyy
Ref #zzz

Add some props, change Ref to See, and we'd nearly have nailed it.


@pento wrote: "I'm really not excited about having this kind of strict format, without the ability to enforce it, and the ability to provide templates."

I'm not sure if there any GitHub apps that offer templates but enforcement can be provided by I beleive https://github.com/paulirish/commitlintbot.

@gziolo
Copy link
Member Author

gziolo commented Aug 3, 2018

I'm really not excited about having this kind of strict format, without the ability to enforce it, and the ability to provide templates.

It's only about merging PRs, something that happens a few times a day usually and is done by a limited number of contributors. What do you propose as an alternative? Enforcing updates to CHANGELOG.md files through reviews?

Thinking about the current WordPress' commit message format:

https://make.wordpress.org/core/handbook/best-practices/commit-messages/

And ideally having this continue once Gutenberg is merged into core rather than the Angular preset, the jQuery preset is very similar to the current WordPress format:

https://github.com/conventional-changelog/conventional-changelog/tree/master/packages/conventional-changelog-jquery

I don't care really what we use as long as it is to follow and doesn't require tons of coding to start using it. It can be jQuery if it is closer to WordPress standards 👍

I'm not sure if there any GitHub apps that offer templates but enforcement can be provided by I beleive https://github.com/paulirish/commitlintbot.

Awesome, that would be great to have it integrated 👍

@youknowriad
Copy link
Contributor

I also don't like enforcing commit messages but I acknowledge that we have an issue we need to solve: How to maintain packages CHANGELOG and since the person releasing packages (which means choosing versions that follow semver) is not the person who did the changes to all the updated packages in general, it means we need a way to know if there's breaking changes, features or just bug fixes when releasing. And a conventional changelog is a "tempting" solution for that.

@aduth
Copy link
Member

aduth commented Aug 3, 2018

Maybe we should revisit the decision for the changelog to be compiled at release time (which we've not really been doing anyways), and move the burden of this onto the developer proposing the change. Then, when it comes time for release, it should be a matter of scanning the changelog and seeing whether changes are breaking or not. This might also need some convention, though even as simple as "Breaking:" (major) and "New:" (minor) and "Fix:" (patch).

@pento
Copy link
Member

pento commented Aug 6, 2018

I'm inclined to agree that the changelog update should happen in the PR that creates the change, rather trying to automate it with fragile scripts.

The pre-release process is then just a matter of deciding whether the items in the unreleased section of the changelog deserve a major, minor, or patch level version bump.

@gziolo
Copy link
Member Author

gziolo commented Aug 6, 2018

I'm inclined to agree that the changelog update should happen in the PR that creates the change, rather trying to automate it with fragile scripts.

I'm fine with that as well and keeping it as simple as @aduth proposed with having PR title prefixed when it is applicable:

"Breaking:" (major) and "New:" (minor) and "Fix:" (patch).

All other PRs should have no prefix for better discoverability. For those 3 types of PRs we would enforce adding an item to the appropriate CHANGELOG.md file. What do you think?

@gziolo
Copy link
Member Author

gziolo commented Aug 8, 2018

We decided to not use conventional commits during Core JS chat yesterday:

Decision: Encourage developers to introduce changelog entries and evaluate success in a future meeting.

See more at https://make.wordpress.org/core/2018/08/07/javascript-chat-summary-august-7/.

@gziolo gziolo closed this Aug 8, 2018
@gziolo gziolo deleted the lerna/conventional-commits branch August 8, 2018 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
npm Packages Related to npm packages [Status] Blocked Used to indicate that a current effort isn't able to move forward [Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants