★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/24030Nouvelle recherche | Portail wanayoo
Skip to content

/node

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

tools: add script to lint first PR commit message #24030

Closed
wants to merge 1 commit into from

Conversation

Reviewers

@refack refack

@Trott Trott

Assignees
No one assigned
Labels
author ready build land-on-v10.x meta tools
Projects
None yet
Milestone
No milestone
6 participants
@richardlau @nodejs-github-bot @Trott @addaleax @refack @codebytere
@richardlau
Copy link
Member

commented Nov 2, 2018

Decouple first commit in pull request linting from Travis by using
the GitHub API to work out the first commit.

The shell script obtains the pull request number in one of the
following ways:

  1. supplied on the command line (use this to test against any PR)
  2. TRAVIS_PULL_REQUEST if set and != "false"
  3. derived from the HEAD commit via the GitHub API

The GitHub API should be more consistent with the web UI in terms of finding
the first commit for a pull request.

This is an alternative to #23758 with the bonus that
you can run the script against any pull request number.

Checklist
@nodejs-github-bot

@nodejs-github-bot nodejs-github-bot added the tools label Nov 2, 2018

@richardlau richardlau referenced this pull request Nov 2, 2018

Closed

tools: add script to lint commit messages #23758

1 of 1 task complete
@richardlau

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Nov 2, 2018

Examples:

Run locally, no pull request specified (script works out based on current HEAD commit):

-bash-4.2$ bash tools/lint-pr-commit-message.sh
Linting the first commit message for pull request 24030
according to the guidelines at https://goo.gl/p2fr5Q.
Commit message for 826115ee852b8aef3dd6e9578c8b0190605c0e98 is:
tools: add script to lint first PR commit message

Decouple first commit in pull request linting from Travis by using
the GitHub API to work out the first commit.

The shell script obtains the pull request number in one of the
following ways:
 1) supplied on the command line (use this to test against any PR)
 2) TRAVIS_PULL_REQUEST if set and != "false"
 3) derived from the HEAD commit via the GitHub API
  ✔  826115ee852b8aef3dd6e9578c8b0190605c0e98
     ✔  0:0      skipping fixes-url                        fixes-url
     ✔  0:0      blank line after title                    line-after-title
     ✔  0:0      line-lengths are valid                    line-length
     ✔  0:0      valid subsystems                          subsystem
     ✔  0:0      Title is formatted correctly.             title-format
     ✔  0:0      Title is <= 50 columns.                   title-length
-bash-4.2$

Checking a completely different PR:

-bash-4.2$ bash tools/lint-pr-commit-message.sh 23758
Linting the first commit message for pull request 23758
according to the guidelines at https://goo.gl/p2fr5Q.
Commit message for fa99a11b207932c6d02ccfc9e8308984ed0f1f76 is:
tools: add script to lint commit messages

This script attempts to guess the target upstream branch.
To manually specify, e.g. to target `canary-base`:
  TARGET_BRANCH=canary-base bash lint-commit-message.sh
  ✔  fa99a11b207932c6d02ccfc9e8308984ed0f1f76
     ✔  0:0      skipping fixes-url                        fixes-url
     ✔  0:0      blank line after title                    line-after-title
     ✔  0:0      line-lengths are valid                    line-length
     ✔  0:0      valid subsystems                          subsystem
     ✔  0:0      Title is formatted correctly.             title-format
     ✔  0:0      Title is <= 50 columns.                   title-length
-bash-4.2$

On Travis (for this PR), https://travis-ci.com/nodejs/node/jobs/155889224:
image
@refack

refack reviewed Nov 2, 2018
View changes

Show resolved Hide resolved tools/lint-pr-commit-message.sh

@refack refack added build meta labels Nov 2, 2018

@refack

refack reviewed Nov 2, 2018
View changes

Show resolved Hide resolved tools/lint-pr-commit-message.sh Outdated
@richardlau
tools: add script to lint first PR commit message 
Decouple first commit in pull request linting from Travis by using
the GitHub API to work out the first commit.

The shell script obtains the pull request number in one of the
following ways:
 1) supplied on the command line (use this to test against any PR)
 2) derived from the HEAD commit via the GitHub API
Loading status checks…
38f2b1e

@richardlau richardlau force-pushed the richardlau:pr-commit-lint branch from 826115e to 38f2b1e Nov 2, 2018

@richardlau

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Nov 2, 2018

Moved the Travis specific stuff out of the script back into .travis.yml.
@richardlau

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Nov 2, 2018

Longer term we could teach core-validate-commit:

  1. to take a PR argument and lint all the commits for that PR (as determined by GitHub's API)
  2. to ignore "fixup!" and "squash!" commit messages

and then replace the shell script here with a direct call to npx -q core-validate-commit ....
@refack

refack approved these changes Nov 2, 2018
View changes

@Trott

Trott approved these changes Nov 4, 2018
View changes

@Trott Trott added the author ready label Nov 4, 2018

@Trott

This comment has been minimized.

Sign in to view
Copy link
Member

commented Nov 4, 2018

Lite CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/1482/

Trott added a commit to Trott/io.js that referenced this pull request Nov 4, 2018

@richardlau @Trott
tools: add script to lint first PR commit message 
Decouple first commit in pull request linting from Travis by using
the GitHub API to work out the first commit.

The shell script obtains the pull request number in one of the
following ways:
 1) supplied on the command line (use this to test against any PR)
 2) derived from the HEAD commit via the GitHub API

PR-URL: nodejs#24030
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
7489ee8
@Trott

This comment has been minimized.

Sign in to view
Copy link
Member

commented Nov 4, 2018 •
edited

Landed in 7489ee8

@Trott Trott closed this Nov 4, 2018

@addaleax

This comment has been minimized.

Sign in to view
Copy link
Member

commented Nov 4, 2018

@Trott @richardlau This seems to have broken Travis CI for non-PR branches without TRAVIS_PULL_REQUEST?

addaleax added a commit to addaleax/node that referenced this pull request Nov 4, 2018

@addaleax
build: do not lint on non-PR Travis builds 
Do not run any linting at all when `TRAVIS_PULL_REQUEST` is `false`.
This would otherwise break Travis CI for `master` and release branches.

Refs: nodejs#24030
Verified
This commit was signed with a verified signature.
@addaleax addaleax Anna Henningsen
GPG key ID: 9C63F3A6CD2AD8F9 Learn about signing commits
6d88351

@addaleax addaleax referenced this pull request Nov 4, 2018

Closed

build: do not lint on non-PR Travis builds #24076

2 of 2 tasks complete

Trott added a commit to Trott/io.js that referenced this pull request Nov 4, 2018

@addaleax @Trott
build: do not lint on non-PR Travis builds 
Do not run any linting at all when `TRAVIS_PULL_REQUEST` is `false`.
This would otherwise break Travis CI for `master` and release branches.

Refs: nodejs#24030

PR-URL: nodejs#24076
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
bde8eb5

@richardlau richardlau deleted the richardlau:pr-commit-lint branch Nov 5, 2018

targos added a commit that referenced this pull request Nov 5, 2018

@richardlau @targos
tools: add script to lint first PR commit message 
Decouple first commit in pull request linting from Travis by using
the GitHub API to work out the first commit.

The shell script obtains the pull request number in one of the
following ways:
 1) supplied on the command line (use this to test against any PR)
 2) derived from the HEAD commit via the GitHub API

PR-URL: #24030
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Verified
This commit was signed with a verified signature.
@targos targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
1541c7f

targos added a commit that referenced this pull request Nov 5, 2018

@addaleax @targos
build: do not lint on non-PR Travis builds 
Do not run any linting at all when `TRAVIS_PULL_REQUEST` is `false`.
This would otherwise break Travis CI for `master` and release branches.

Refs: #24030

PR-URL: #24076
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Verified
This commit was signed with a verified signature.
@targos targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
3de1c5c

@richardlau richardlau referenced this pull request Nov 6, 2018

Closed

Travis linter false-negatives for commit message checks #23737

@BridgeAR BridgeAR referenced this pull request Nov 14, 2018

Merged

Release proposal: v11.2 #24350

This was referenced Nov 15, 2018

Closed
Closed

This was referenced Nov 15, 2018

Closed
Closed
Closed
Closed

codebytere added a commit that referenced this pull request Nov 29, 2018

@richardlau @codebytere
tools: add script to lint first PR commit message 
Decouple first commit in pull request linting from Travis by using
the GitHub API to work out the first commit.

The shell script obtains the pull request number in one of the
following ways:
 1) supplied on the command line (use this to test against any PR)
 2) derived from the HEAD commit via the GitHub API

PR-URL: #24030
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Verified
This commit was signed with a verified signature.
@codebytere codebytere Shelley Vohr
GPG key ID: F13993A75599653C Learn about signing commits
Loading status checks…
2f1fd0e

codebytere added a commit that referenced this pull request Nov 29, 2018

@addaleax @codebytere
build: do not lint on non-PR Travis builds 
Do not run any linting at all when `TRAVIS_PULL_REQUEST` is `false`.
This would otherwise break Travis CI for `master` and release branches.

Refs: #24030

PR-URL: #24076
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Verified
This commit was signed with a verified signature.
@codebytere codebytere Shelley Vohr
GPG key ID: F13993A75599653C Learn about signing commits
Loading status checks…
3bba05c

@codebytere codebytere added the land-on-v10.x label Nov 29, 2018

MylesBorins added a commit that referenced this pull request Nov 29, 2018

@richardlau @MylesBorins
tools: add script to lint first PR commit message 
Decouple first commit in pull request linting from Travis by using
the GitHub API to work out the first commit.

The shell script obtains the pull request number in one of the
following ways:
 1) supplied on the command line (use this to test against any PR)
 2) derived from the HEAD commit via the GitHub API

PR-URL: #24030
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Verified
This commit was signed with a verified signature.
@MylesBorins MylesBorins Myles Borins
GPG key ID: 933B01F40B5CA946 Learn about signing commits
0ad9103

MylesBorins added a commit that referenced this pull request Nov 29, 2018

@addaleax @MylesBorins
build: do not lint on non-PR Travis builds 
Do not run any linting at all when `TRAVIS_PULL_REQUEST` is `false`.
This would otherwise break Travis CI for `master` and release branches.

Refs: #24030

PR-URL: #24076
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Verified
This commit was signed with a verified signature.
@MylesBorins MylesBorins Myles Borins
GPG key ID: 933B01F40B5CA946 Learn about signing commits
9b55b90

@codebytere codebytere referenced this pull request Nov 29, 2018

Merged

v10.14.2 proposal #24727

MylesBorins added a commit that referenced this pull request Dec 3, 2018

@richardlau @MylesBorins
tools: add script to lint first PR commit message 
Decouple first commit in pull request linting from Travis by using
the GitHub API to work out the first commit.

The shell script obtains the pull request number in one of the
following ways:
 1) supplied on the command line (use this to test against any PR)
 2) derived from the HEAD commit via the GitHub API

PR-URL: #24030
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Verified
This commit was signed with a verified signature.
@MylesBorins MylesBorins Myles Borins
GPG key ID: 933B01F40B5CA946 Learn about signing commits
1538148

MylesBorins added a commit that referenced this pull request Dec 3, 2018

@addaleax @MylesBorins
build: do not lint on non-PR Travis builds 
Do not run any linting at all when `TRAVIS_PULL_REQUEST` is `false`.
This would otherwise break Travis CI for `master` and release branches.

Refs: #24030

PR-URL: #24076
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Verified
This commit was signed with a verified signature.
@MylesBorins MylesBorins Myles Borins
GPG key ID: 933B01F40B5CA946 Learn about signing commits
3bac885
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.