nodejs
feat: add `--validate-no-metadata` flag
Useful to ensure the commit message doesn't include metadata that should be added by automation (PR-URL and reviewers). In particular, we could use that in the commit linter in nodejs/node to report as invalid commits that include a PR-URL or Reviewed-by trailers so the CQ refuse to land such commit as having those manually entered is error prone.
No objection on the flag, but I'm not sure we will be able to add it to nodejs/node. There are valid commits with metadata:
- backports
- cherry-picks of floating patches (on V8 for example)
We don't land backports with the CQ, so I don't think that would be problematic. For cherry-picks, I don't think the commit would contain PR-URL or Reviewed-by trailers, would they? If not, then this flag won't cause a problem since it's only checking for those two.
It does. At least for V8 patches, I always keep the metadata for the first time a commit landed. For example: https://github.com/nodejs/node/commit/cc36db7c06a16d2e9ae34b94f2e0d4d0c39a4168
I see! So if we started to use that flag in Node.js, we would need to land those PRs manually. Would that disrupt your process?
I guess it would actually not apply for those, since the GHA commit linter is only validating the first commit message, and those floating patches are never first IIRC.
I guess it would actually not apply for those, since the GHA commit linter is only validating the first commit message, and those floating patches are never first IIRC.
They are on, e.g. https://github.com/nodejs/node/pull/53997.
I guess it would actually not apply for those, since the GHA commit linter is only validating the first commit message, and those floating patches are never first IIRC.
They are on, e.g. nodejs/node#53997.
I added a test with the commit on the PR you linked, it still passes with the new flag.