tools: apply linting to first commit in PRs

[Trott]

First commit:

tools: only use Travis-CI on master branch

Limit Travis-CI to the master branch. One benefit is that it will
simplify the logic for commit message linting.

Second commit:

tools: apply linting to first commit in PRs

Use Travis-CI to check the formatting of the first commit in a pull
request. This will hopefully reduce formatting errors and nits about
them in pull requests.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

[nodejs-github-bot]

@Trott build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/623/pipeline

[Trott]

Aw yeah...

[Trott]

Since this alters .travis.yml and nothing else, the pipeline job and the Travis run should be sufficient rather than a full CI.

[mscdex]

Why only the first commit?

[Trott]

Why only the first commit?

Because subsequent commits are often autosquash commits and I don't want to discourage people from using that. Maybe in the very near future, if we can create a CLI flag for core-validate-commit that skips commits that start with squash! or fixup!, we can lint all the commits. /cc @evanlucas @nodejs/automation

[mscdex]

Is there no way to have CI try to auto squash all of the commits first?

[Trott]

Is there no way to have CI try to auto squash all of the commits first?

Oooh, I do like that idea! The --autosquash flag is only honored for interactive rebases. There's no doubt some way to make interactive mode not need to be interactive (such as using expect although there are probably even easier ways) but I'm not sure I want to go down that potentially gotcha-ridden rabbit hole before landing this. I'd prefer this land as it is now, and then pursue that as a subsequent enhancement.

[targos]

I would like to avoid the first commit if possible. As someone who works a lot with canary and release branches, having Travis run on them is very useful

[Trott]

I would like to avoid the first commit if possible. As someone who works a lot with canary and release branches, having Travis run on them is very useful

OK. I wasn't aware that it was unusually valuable in that context. I'll see if I can figure something out after I sleep and attend the TSC meeting. :-D I'll remove the fast-track label and mark this as in progress. If someone knows how to make it work for everything, by all means, PR into the branch or commit right to it. Thanks.

[refack]

I would like to avoid the first commit if possible. As someone who works a lot with canary and release branches, having Travis run on them is very useful

This has been enabled only for PR on master, and with --no-validate-metadata, it might actually be useful even for release and canary branches. It's validates only the common sense rules:

f918801e0fd1b89531e9230c985cadea91914a6e
     ✔  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

Once we teach core-validate-commit to ignore commits that start with squash! or fixup! IMHO it could work for all commits (in PRs on master)

[targos]

@refack my comment wasn't about core-validate-commit. What I care about is that Travis CI runs when commits are pushed to canary-base, v10.x-staging, v10.x, etc

[refack]

Could you add a before_install: stage:

before_install: npm i -g core-validate-commit

instead of the npx call.
I find this more readable (one less magic word in L18)

[refack]

@refack my comment wasn't about core-validate-commit. What I care about is that Travis CI runs when commits are pushed to canary-base, v10.x-staging, v10.x, etc

Ahh the first commit of this PR, not the first commit rule 🤦‍♂️

[BridgeAR]

Any progress here?

[Trott]

Rebased/force-pushed to eliminate conflict.

Will get back to this soon, I think...