chore(commit message): use commitplease to validate commit messages #9953

alisianoi · 2016-07-11T12:24:51Z

Please check if the PR fulfills these requirements

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit-message-format
~~Tests for the changes have been added (for bug fixes / features)~~
~~Docs have been added / updated (for bug fixes / features)~~

What kind of change does this PR introduce? (check one with "x")

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[x] Other... Please describe: check that commit messages adhere to style guide

What is the current behavior? (You can also link to an open issue here)

Developers follow commit message style best they can

What is the new behavior?

An external npm module that checks commit messages for you

Does this PR introduce a breaking change? (check one with "x")

[ ] Yes
[x] No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

commitplease is an npm module that installs a git hook during npm install and checks that your commit message follows the style guide when you do git commit. If the message does not follow the style guide, the commit is aborted and an exact diagnostic is printed so that the developer can correct the commit message.

How to try out:

git checkout commitplease
mv .git/hooks/commit-msg .git/hooks/commit-msg.old
npm install

Now try to commit some changes with valid/invalid commit messages.

How to go back:

git checkout master
mv .git/hooks/commit-msg.old .git/hooks/commit-msg

Disclaimer: I am contributing to commitplease as part of the Google Summer of Code project for students.

There is also a twin pull request to angular/angular.js#14888

vicb · 2016-07-11T15:30:13Z

I often commit tmp commits during dev, would it be possible to

only prevent pushes
have a way to force a push

alisianoi · 2016-07-11T15:56:55Z

For temporary commits, you could either begin the commit message with:

WIP, wip or Wip which usually stands for "Work in progress"
fixup! or squash! which is a way to tell git to squash the commits later

or you could do git commit --no-verify which does not trigger the git hook altogether (does not run the validation script at all)

alexeagle · 2016-07-25T23:08:11Z

package.json

@@ -115,5 +116,8 @@
    "webpack": "^1.12.6",
    "which": "~1",
    "yargs": "^3.31.0"
+  },
+  "commitplease": {


should we include types as well?
https://www.npmjs.com/package/commitplease#angularjs
they are listed here:
https://github.com/angular/angular/blob/master/CONTRIBUTING.md#type

no, those types are actually default values inside commitplease (once you choose angular style) and so can be left out

But you could redefine them in package.json if needed, as shown in your first link

I usually use WIP at the beginning of a tmp commit message, so this seems
like a good idea to me. Any objection @vicb ?

On Mon, Jul 25, 2016 at 11:17 PM Alexander Lisianoi <
notifications@github.com> wrote:

In package.json
#9953 (comment):

@@ -115,5 +116,8 @@
"webpack": "^1.12.6",
"which": "~1",
"yargs": "^3.31.0"

},

"commitplease": {

no, those types are actually default values inside commitplease and can be
left out

But you could redefine them in package.json if needed, as shown in the
your link

—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
https://github.com/angular/angular/pull/9953/files/7b6721838c8a96fedeabc08b5be7c931e19f459f#r72192975,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAC5I-aDiQk4z01m-oEbhPbkAa95hB7gks5qZaZ0gaJpZM4JJT0y
.

vicb · 2016-07-28T16:26:08Z

Can we use "f!" fixup is too long ?

I'm 👎 to merge this before:

It is documented in the CONTRIBUTOR or DEVELOPPER guide (don't remember the best fir on top of my head) - how to escape from the message dictature, ...
The error message points to this doc.

I think it would be a really bad dev XP to have a cryptic error message on commits. (Not that I know the current one but that's related to my previous point)

vicb · 2016-12-10T01:24:17Z

@all3fox sorry for the delay in processing.

I would be interested in merging this PR.

Is there a way to test the commits message on the CI ? (fails if the commit message do not follow our conventions)

Thanks

alisianoi · 2016-12-10T09:13:18Z

Hello @vicb

Currently, commitplease checks its argv[0] which can be (here is the code):

The .git/COMMIT_EDITMSG path supplied by the commit-msg hook. Then commitplease reads the contents of this file and validates them.
Something that can be used by git log to filter commits (like master to show that branch or master..feature to show new commits on branch feature, more examples here)

So, for instance you could have a shell script that creates a git repository, installs commitlpease there, configures it however you like via package.json and then begins the cycle of adding commits and reading git responses.

Finally, on our Travis CI we use this test.js script that basically creates a bunch of messageX -- profilesX pairs of lists. Each such pair corresponds to a particular style of commits (message0 and profile0 are for jQuery, message1 and profile1 are for Angular, message9 and profile9 are for common parts). By default, a profile from profileX must successfully validate all the messages from messageX. Alternatively, for every message you could say that the profile must be in rejects or reasons, which means that the profile simply has to reject the message or reject it and print a specific error message. And you could also cherry-pick a subset of tests, like with jquery4 profile here (it is not in any profilesX list but is manually inserted in some places)

So, what I am saying is you could take the main test function from line 802 and then roll your own Travis CI test script with messageX --- profilesX lists.

Tell me what you would prefer and I may help with that.

jzaefferer · 2016-12-10T10:20:20Z

You should also be able to call the commitplease executable in a local install on Travis as described here: https://github.com/jzaefferer/commitplease/blob/master/README.md#global-install

@matsko

This patch adds the gulp command of `validate-commit-messages` which will validate the range of commits messages present in the active branch. This check now runs on CI as part of the linting checks. Allowed commit message types and scopes are controlled via commit-message.json file and documented at https://github.com/angular/angular/blob/master/CONTRIBUTING.md#-commit-message-guidelines This solution is based on old Vojta's code that he wrote for angular/angular.js, that was later adjusted by @matsko in angular#13815. Ideally we should switch over to something like https://www.npmjs.com/package/commitplease as suggested in angular#9953 but that package currently doesn't support strict scope checking, which is one of the primarily goal of this PR. Note that this PR removes support for "chore" which was previously overused by everyone on the team. Closes angular#13815 Fixes angular#3337

@matsko

This patch adds the gulp command of `validate-commit-messages` which will validate the range of commits messages present in the active branch. This check now runs on CI as part of the linting checks. Allowed commit message types and scopes are controlled via commit-message.json file and documented at https://github.com/angular/angular/blob/master/CONTRIBUTING.md#-commit-message-guidelines This solution is based on old Vojta's code that he wrote for angular/angular.js, that was later adjusted by @matsko in #13815. Ideally we should switch over to something like https://www.npmjs.com/package/commitplease as suggested in #9953 but that package currently doesn't support strict scope checking, which is one of the primarily goal of this PR. Note that this PR removes support for "chore" which was previously overused by everyone on the team. Closes #13815 Fixes #3337

alxhub · 2017-01-25T23:24:58Z

Closing as outdated, please sync and re-open if needed.

@matsko

This patch adds the gulp command of `validate-commit-messages` which will validate the range of commits messages present in the active branch. This check now runs on CI as part of the linting checks. Allowed commit message types and scopes are controlled via commit-message.json file and documented at https://github.com/angular/angular/blob/master/CONTRIBUTING.md#-commit-message-guidelines This solution is based on old Vojta's code that he wrote for angular/angular.js, that was later adjusted by @matsko in angular#13815. Ideally we should switch over to something like https://www.npmjs.com/package/commitplease as suggested in angular#9953 but that package currently doesn't support strict scope checking, which is one of the primarily goal of this PR. Note that this PR removes support for "chore" which was previously overused by everyone on the team. Closes angular#13815 Fixes angular#3337

angular-automatic-lock-bot · 2019-09-10T16:21:31Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes label Jul 11, 2016

vicb added the action: discuss label Jul 11, 2016

This was referenced Jul 19, 2016

What about checking commit message style with commitplease? eslint/eslint#6706

Closed

Add common objections and suggested solutions to readme jzaefferer/commitplease#67

Closed

alexeagle reviewed Jul 25, 2016
View reviewed changes

alexeagle assigned alexeagle and unassigned alexeagle Jul 25, 2016

chore(commit message): use commitplease to validate commit messages

63df107

vsavkin added the area: build & ci Related the build and CI infrastructure of the project label Oct 4, 2016

vicb self-assigned this Dec 8, 2016

vicb mentioned this pull request Jan 6, 2017

chore(ci): validate the message of each new commit during ci #13815

Closed

alxhub closed this Jan 25, 2017

marionebl mentioned this pull request Nov 20, 2017

ci: use commitlint to validate messages #20521

Closed

3 tasks

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore(commit message): use commitplease to validate commit messages #9953

chore(commit message): use commitplease to validate commit messages #9953

Uh oh!

alisianoi commented Jul 11, 2016

Uh oh!

vicb commented Jul 11, 2016

Uh oh!

alisianoi commented Jul 11, 2016 •

edited

Loading

Uh oh!

alexeagle Jul 25, 2016

Uh oh!

alisianoi Jul 26, 2016 •

edited

Loading

Uh oh!

alexeagle Jul 26, 2016

Uh oh!

vicb commented Jul 28, 2016 •

edited

Loading

Uh oh!

vicb commented Dec 10, 2016

Uh oh!

alisianoi commented Dec 10, 2016 •

edited

Loading

Uh oh!

jzaefferer commented Dec 10, 2016

Uh oh!

alxhub commented Jan 25, 2017

Uh oh!

angular-automatic-lock-bot bot commented Sep 10, 2019

Uh oh!

Uh oh!

chore(commit message): use commitplease to validate commit messages #9953

chore(commit message): use commitplease to validate commit messages #9953

Uh oh!

Conversation

alisianoi commented Jul 11, 2016

Uh oh!

vicb commented Jul 11, 2016

Uh oh!

alisianoi commented Jul 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexeagle Jul 25, 2016

Choose a reason for hiding this comment

Uh oh!

alisianoi Jul 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexeagle Jul 26, 2016

Choose a reason for hiding this comment

Uh oh!

vicb commented Jul 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vicb commented Dec 10, 2016

Uh oh!

alisianoi commented Dec 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jzaefferer commented Dec 10, 2016

Uh oh!

alxhub commented Jan 25, 2017

Uh oh!

angular-automatic-lock-bot bot commented Sep 10, 2019

Uh oh!

Uh oh!

alisianoi commented Jul 11, 2016 •

edited

Loading

alisianoi Jul 26, 2016 •

edited

Loading

vicb commented Jul 28, 2016 •

edited

Loading

alisianoi commented Dec 10, 2016 •

edited

Loading