tools: add script to lint commit messages #23758
Conversation
tools/lint-commit-message.sh
Outdated
| MINOR_VERSION=$( grep '#define NODE_MINOR_VERSION [0-9][0-9]*' "${ROOT_DIR}/src/node_version.h" | awk '{print $(NF)}' ) | ||
| PATCH_VERSION=$( grep '#define NODE_PATCH_VERSION [0-9][0-9]*' "${ROOT_DIR}/src/node_version.h" | awk '{print $(NF)}' ) | ||
| # If the version is not in the CHANGELOG assume the target branch is master | ||
| if grep "${MAJOR_VERSION}\.${MINOR_VERSION}\.${PATCH_VERSION}" "${ROOT_DIR}/CHANGELOG.md" > /dev/null; then |
tools/lint-commit-message.sh
Outdated
| echo "Target branch is ${TARGET_BRANCH}" | ||
|
|
||
| # Make sure we're up to date | ||
| UPSTREAM=$( git remote -v | grep "github\.com.*nodejs/node (fetch)" | awk '{print $(1)}' ) |
There was a problem hiding this comment.
What if there is no such remote?
There was a problem hiding this comment.
Good question. I think that's why the Travis build failed 😆 .
|
Haha, that didn't work on Travis: |
|
Fixed Travis: |
mmarchini
added
the
author ready
tools/lint-commit-message.sh
Outdated
| echo "Fetching upstream remote ${UPSTREAM}" | ||
| git fetch "${UPSTREAM}" | ||
| COMMON_ANCESTOR=$( git merge-base "${UPSTREAM}/$TARGET_BRANCH" HEAD ) | ||
| COMMITS=$( git rev-list --no-merges ${COMMON_ANCESTOR}..HEAD | tail -1 ) |
There was a problem hiding this comment.
Maybe use FIRST_COMMIT (or at least singular COMMIT) for clarity?
tools/lint-commit-message.sh
Outdated
| COMMITS=$( git rev-list --no-merges ${COMMON_ANCESTOR}..HEAD | tail -1 ) | ||
|
|
||
| if [ -n "${COMMITS}" ]; then | ||
| echo "Linting the commit message according to the guidelines at https://goo.gl/p2fr5Q" |
There was a problem hiding this comment.
I'd note that it only lints first commit message. (It'd stop those who don't know that from wasting their time on fixing every commit they have in the branch)
There was a problem hiding this comment.
This is the exact message currently on Travis: #23742
I'll change it here.
tools/lint-commit-message.sh
Outdated
| echo "$( git log -1 ${COMMITS} )" | ||
| echo "${COMMITS}" | xargs npx -q core-validate-commit --no-validate-metadata | ||
| else | ||
| echo "No commits found to lint." |
There was a problem hiding this comment.
Nit: there is a dot at the end, so I'd suggest to either have the dot in every echo message or in none, just for consistency.
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
richardlau
force-pushed
the
commit-lint
branch
from
2fca0dd
to
bc52f0a
Compare
richardlau
added
wip
|
Addressed nits, but it looks like in the meantime my branch and master have diverged and this is now failing to properly pick up the first commit. Removed |
|
After thinking about this a bit more, I've come up with an alternative: #24030 |
Currently the "first PR commit message" linting is only done on Travis. This PR attempts to
decouple the logic from Travis and capture the logic in a bash shell script so that it can be
run locally.
I haven't (yet) added the scripts to
Makefileorvcbuild.batas the script requires git, node,npm and npx being in $PATH.
Refs: #22452
Checklist