You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add core-validate-commit to node-test-pull-request #793
I use it as a sanity check before landing. It'll fail before then anyway as it won't find the PR-URL or Reviewed-By lines. I think @refack has it set up as a pre-commit hook (so you can't commit without it being correct), though I could be misremembering.
I am -1 with this change as proposed. This will fail for all the PRs on the first go, because they lack reviews and a PR url. It will just increase the amount of nit-picking, as we will likely require to fix those details by the author, while we currently fix them on landing.
If we can disable those checks for the job, then I am +1.
pre-push, and it has a if ($2.includes('/nodejs/node')).
I don't want to tie my hands completely.
P.S. if it could be added as a warning to the linter job that might be better. Or if at some point we make a node-land job (could take a forced-pushed PR or an arbitrary branch in the landers repo) that's the more appropriate place.
It'll fail before then anyway as it won't find the PR-URL or Reviewed-By lines.
You can pass the --no-validate-metadata flag and it won't fail from a missing PR-URL or missing Reviewed-By.
The job that is there works pretty well. It can be run against a single commit or all of the commits for a PR. It becomes tricky with fixup commits though...
We are running core-validate-commit in Travis CI but it was recently marked as allowed to fail which means that you need to manually look at the Travis CI results (either the check tab on the PR page or in Travis itself) to see if it failed or not (the overall check status for Travis CI in the main tab on the PR page will be green even if the core-validate-commit run fails).
Activity
n-api: Implement stricter wrapping
joaocgreis commentedon Jul 13, 2017
+1
I remember some discussion around this, but I can't find it. @evanlucas do you remember? If it was simply not ready at the time, is it now?
gibfahn commentedon Jul 13, 2017
The job is there, it just needs to be included (as long as it's working). I see that @evanlucas has access to it.
https://ci.nodejs.org/job/node-test-commitmsg/
gibfahn commentedon Jul 13, 2017
@mhdawson in reply to nodejs/node@d5b397c#commitcomment-23061905
I use it as a sanity check before landing. It'll fail before then anyway as it won't find the
PR-URLorReviewed-Bylines. I think @refack has it set up as a pre-commit hook (so you can't commit without it being correct), though I could be misremembering.mcollina commentedon Jul 13, 2017
I am -1 with this change as proposed. This will fail for all the PRs on the first go, because they lack reviews and a PR url. It will just increase the amount of nit-picking, as we will likely require to fix those details by the author, while we currently fix them on landing.
If we can disable those checks for the job, then I am +1.
refack commentedon Jul 13, 2017
pre-push, and it has aif ($2.includes('/nodejs/node')).I don't want to tie my hands completely.
P.S. if it could be added as a warning to the linter job that might be better. Or if at some point we make a
node-landjob (could take a forced-pushed PR or an arbitrary branch in the landers repo) that's the more appropriate place.evanlucas commentedon Jul 13, 2017
You can pass the
--no-validate-metadataflag and it won't fail from a missing PR-URL or missing Reviewed-By.The job that is there works pretty well. It can be run against a single commit or all of the commits for a PR. It becomes tricky with fixup commits though...
fhinkel commentedon Jul 14, 2017
core-validate-commitis great! But iIrc it complains about Chromium backports because a line with the URL is too long.evanlucas commentedon Jul 14, 2017
@fhinkel I’ll try to get around to fixing that.
sam-github commentedon Jan 28, 2020
Closing as stale, but if anyone wants to take this up feel free to reopen.
richardlau commentedon Jan 28, 2020
We are running
core-validate-commitin Travis CI but it was recently marked as allowed to fail which means that you need to manually look at the Travis CI results (either the check tab on the PR page or in Travis itself) to see if it failed or not (the overall check status for Travis CI in the main tab on the PR page will be green even if thecore-validate-commitrun fails).