tools: change text about Travis

[aymen94]

Changed ambiguous text with clear text.

Checklist

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

[richardlau]

I'm not going to block this, but I'm of the opinion this is less clear than what was there before. The job name shows up in the Travis checks and are basically statements with pass/fail states:

The new name is no longer a statement and instead is a request/instruction. It also now drops the "first commit" which indicated that currently only the first commit is message linted.

[aymen94]

If you update your PR, adding other commit travis recheck the last commit added

[richardlau]

No, Travis always runs when the branch the pull request is created from is updated, but the commit message check always checks the first commit (and currently none of the subsequent ones).

[refack]

Maybe just

Suggested change

	- name: "Please follow commit message guidelines: <a href=\"https://goo.gl/p2fr5Q\">link</a>"
	- name: "First commit message adheres to guidelines at <a href=\"https://goo.gl/p2fr5Q\">[this link]</a>"

To hide the slug and avoid the line wrap

[Trott]

Oh, yeah, I didn't even notice that this was the job name rather than text displayed on error. I agree the current text is better.

[Trott]

No, Travis always runs when the branch the pull request is created from is updated, but the commit message check always checks the first commit (and currently none of the subsequent ones).

And, for the record, that is a design choice rather than a bug. Subsequent commits are usually squashed into the first commit so that's the one that needs a valid message in most cases.

[aymen94]

I think it's better to check the last commit, and the previous commits crush them on the last because the last is definitive

[Trott]

I think it's better to check the last commit, and the previous commits crush them on the last because the last is definitive

No. The commit message of the first message is almost always the one to check. Subsequent commits are often/usually fixup commits.

No strong opinion either way.

[BridgeAR]

@aymen94 to me the concerns @richardlau and @Trott brought up seem valid. I understand why you opened the PR but do you still feel the new wording is clearer due to the way it is mainly viewed?

[aymen94]

@BridgeAR @richardlau @Trott
what do you think about it?
The first commit message complies with the guidelines: link

if it's not okay, I close the PR 😃

[BridgeAR]

We should keep on using the actual link instead of link. Otherwise it's not possible to copy that part from the overview.

Besides that I am fine with the suggestion.

[BridgeAR]

@aymen94 would you be so kind and rebase this branch with the current master and force push? Otherwise we can not run the CI.

[aymen94]

@BridgeAR Of course.

[BridgeAR]

@aymen94 there were still merge commits on your branch. I just went ahead and rebased this. You can find further information about rebasing here: https://git-scm.com/book/en/v2/Git-Branching-Rebasing

You also find a lot of documentation about our process here: https://github.com/nodejs/node/blob/master/doc/guides/contributing/pull-requests.md

[BridgeAR]

CI https://ci.nodejs.org/job/node-test-pull-request/21711/

[Trott]

Not going to block it but it's not clear to me how this is an improvement? It seems to change the wording, but doesn't make it any better or worse. Am I missing something?

[BridgeAR]

@Trott not really... I have no strong opinion about either wording. They both seem fine to me.

Is either one maybe "more intuitive" to some people? Maybe we can just gather some feedback:

❤️ for the current one
🎉 for the suggested one

[Trott]

I voted for the "current" wording not because it's better but because I don't want changes landing if they aren't believed to be improvements. But again, not blocking. And not saying the current wording is better. Just not sure it really makes any significant difference either way.

[aymen94]

I suggested the change if it can give problems I close the PR.

[BridgeAR]

@aymen94 I am going to close this PR due to the votes for keeping it as it is.

Thanks a lot for your contribution nevertheless!