feat: check large pull request approvals in git node land

[zeexzeex]

Node.js requires two TSC approvals for large pull requests, but nothing enforces it at land time. This adds that check to git node land.

Whether a pull request is large is not something that can be derived from the diff. The policy counts a new subsystem as large regardless of size, and exempts routine dependency updates but not other dependency changes; neither is visible in the pull request data. An earlier revision of this PR tried a 5000 line threshold, and checking it against the 6 pull requests currently carrying the large-pr label, it missed 2 of them: nodejs/node#64429 is well under the threshold, and nodejs/node#62217 has a net negative diff.

The project already records this judgement with the large-pr label, so this checks that instead, the way semver-major is checked. The approval requirement is the same for both, so the existing branch is widened rather than duplicated, with a distinct message and reason code so a failure tells the author which rule applies. A pull request carrying both labels is reported as semver-major.

Refs: https://github.com/nodejs/node/blob/main/doc/contributing/large-pull-requests.md
Fixes: #1063

[Renegade334]

I don't think that trying to automatically detect whether a PR falls into the policy definition of a large PR is ever going to be feasible. This can't detect whether a PR adds a new subsystem, or detect whether a non-automated dependency change PR meets the large PR threshold (it's only automated commits that are exempt), and indeed a PR only needs to touch a single gypfile in /deps for this logic to exempt it.

I think we would be much better off making this a label-based check, à la fast track or semver-major. Large PRs are not getting landed quickly, it's highly likely that a clued-in reviewer will check in to add the label long before it's in a position to land, and we can always couple it with an action to conditionally comment a reminder when the PR is opened.

[zeexzeex]

You're right. I checked against the large-pr label: of the 6 PRs carrying it, my logic misses 2. #64429 is well under the threshold, and #62217 has a net negative diff plus the dependencies label, so it gets exempted twice over despite not being a routine update.

I'll rewrite this as a large-pr label check alongside the existing semver-major branch, and drop the line counting, the exemption list, and the additions/deletions query fields.

[zeexzeex]

Node.js requires two TSC approvals for large pull requests, but nothing enforces it at land time. This adds that check to git node land.

Whether a pull request is large is not something that can be derived from the diff. The policy counts a new subsystem as large regardless of size, and exempts routine dependency updates but not other dependency changes; neither is visible in the pull request data. An earlier revision of this PR tried a 5000 line threshold, and checking it against the 6 pull requests currently carrying the large-pr label, it missed 2 of them: nodejs/node#64429 is well under the threshold, and nodejs/node#62217 has a net negative diff.

The project already records this judgement with the large-pr label, so this checks that instead, the way semver-major is checked. The approval requirement is the same for both, so the existing branch is widened rather than duplicated, with a distinct message and reason code so a failure tells the author which rule applies. A pull request carrying both labels is reported as semver-major.

Refs: https://github.com/nodejs/node/blob/main/doc/contributing/large-pull-requests.md
Fixes: #1063

[Renegade334]

I don't think that trying to automatically detect whether a PR falls into the policy definition of a large PR is ever going to be feasible. This can't detect whether a PR adds a new subsystem, or detect whether a non-automated dependency change PR meets the large PR threshold (it's only automated commits that are exempt), and indeed a PR only needs to touch a single gypfile in /deps for this logic to exempt it.

I think we would be much better off making this a label-based check, à la fast track or semver-major. Large PRs are not getting landed quickly, it's highly likely that a clued-in reviewer will check in to add the label long before it's in a position to land, and we can always couple it with an action to conditionally comment a reminder when the PR is opened.

[zeexzeex]

You're right. I checked against the large-pr label: of the 6 PRs carrying it, my logic misses 2. #64429 is well under the threshold, and #62217 has a net negative diff plus the dependencies label, so it gets exempted twice over despite not being a routine update.

I'll rewrite this as a large-pr label check alongside the existing semver-major branch, and drop the line counting, the exemption list, and the additions/deletions query fields.

[zeexzeex]

Node.js requires two TSC approvals for large pull requests, but nothing enforces it at land time. This adds that check to git node land.

Whether a pull request is large is not something that can be derived from the diff. The policy counts a new subsystem as large regardless of size, and exempts routine dependency updates but not other dependency changes; neither is visible in the pull request data. An earlier revision of this PR tried a 5000 line threshold, and checking it against the 6 pull requests currently carrying the large-pr label, it missed 2 of them: nodejs/node#64429 is well under the threshold, and nodejs/node#62217 has a net negative diff.

The project already records this judgement with the large-pr label, so this checks that instead, the way semver-major is checked. The approval requirement is the same for both, so the existing branch is widened rather than duplicated, with a distinct message and reason code so a failure tells the author which rule applies. A pull request carrying both labels is reported as semver-major.

Refs: https://github.com/nodejs/node/blob/main/doc/contributing/large-pull-requests.md
Fixes: #1063

[Renegade334]

I don't think that trying to automatically detect whether a PR falls into the policy definition of a large PR is ever going to be feasible. This can't detect whether a PR adds a new subsystem, or detect whether a non-automated dependency change PR meets the large PR threshold (it's only automated commits that are exempt), and indeed a PR only needs to touch a single gypfile in /deps for this logic to exempt it.

I think we would be much better off making this a label-based check, à la fast track or semver-major. Large PRs are not getting landed quickly, it's highly likely that a clued-in reviewer will check in to add the label long before it's in a position to land, and we can always couple it with an action to conditionally comment a reminder when the PR is opened.

[zeexzeex]

You're right. I checked against the large-pr label: of the 6 PRs carrying it, my logic misses 2. #64429 is well under the threshold, and #62217 has a net negative diff plus the dependencies label, so it gets exempted twice over despite not being a routine update.

I'll rewrite this as a large-pr label check alongside the existing semver-major branch, and drop the line counting, the exemption list, and the additions/deletions query fields.

[zeexzeex]

Node.js requires two TSC approvals for large pull requests, but nothing enforces it at land time. This adds that check to git node land.

Whether a pull request is large is not something that can be derived from the diff. The policy counts a new subsystem as large regardless of size, and exempts routine dependency updates but not other dependency changes; neither is visible in the pull request data. An earlier revision of this PR tried a 5000 line threshold, and checking it against the 6 pull requests currently carrying the large-pr label, it missed 2 of them: nodejs/node#64429 is well under the threshold, and nodejs/node#62217 has a net negative diff.

The project already records this judgement with the large-pr label, so this checks that instead, the way semver-major is checked. The approval requirement is the same for both, so the existing branch is widened rather than duplicated, with a distinct message and reason code so a failure tells the author which rule applies. A pull request carrying both labels is reported as semver-major.

Refs: https://github.com/nodejs/node/blob/main/doc/contributing/large-pull-requests.md
Fixes: #1063

[Renegade334]

I don't think that trying to automatically detect whether a PR falls into the policy definition of a large PR is ever going to be feasible. This can't detect whether a PR adds a new subsystem, or detect whether a non-automated dependency change PR meets the large PR threshold (it's only automated commits that are exempt), and indeed a PR only needs to touch a single gypfile in /deps for this logic to exempt it.

I think we would be much better off making this a label-based check, à la fast track or semver-major. Large PRs are not getting landed quickly, it's highly likely that a clued-in reviewer will check in to add the label long before it's in a position to land, and we can always couple it with an action to conditionally comment a reminder when the PR is opened.

[zeexzeex]

You're right. I checked against the large-pr label: of the 6 PRs carrying it, my logic misses 2. #64429 is well under the threshold, and #62217 has a net negative diff plus the dependencies label, so it gets exempted twice over despite not being a routine update.

I'll rewrite this as a large-pr label check alongside the existing semver-major branch, and drop the line counting, the exemption list, and the additions/deletions query fields.

[zeexzeex]

Node.js requires two TSC approvals for large pull requests, but nothing enforces it at land time. This adds that check to git node land.

Whether a pull request is large is not something that can be derived from the diff. The policy counts a new subsystem as large regardless of size, and exempts routine dependency updates but not other dependency changes; neither is visible in the pull request data. An earlier revision of this PR tried a 5000 line threshold, and checking it against the 6 pull requests currently carrying the large-pr label, it missed 2 of them: nodejs/node#64429 is well under the threshold, and nodejs/node#62217 has a net negative diff.

The project already records this judgement with the large-pr label, so this checks that instead, the way semver-major is checked. The approval requirement is the same for both, so the existing branch is widened rather than duplicated, with a distinct message and reason code so a failure tells the author which rule applies. A pull request carrying both labels is reported as semver-major.

Refs: https://github.com/nodejs/node/blob/main/doc/contributing/large-pull-requests.md
Fixes: #1063

[Renegade334]

I don't think that trying to automatically detect whether a PR falls into the policy definition of a large PR is ever going to be feasible. This can't detect whether a PR adds a new subsystem, or detect whether a non-automated dependency change PR meets the large PR threshold (it's only automated commits that are exempt), and indeed a PR only needs to touch a single gypfile in /deps for this logic to exempt it.

I think we would be much better off making this a label-based check, à la fast track or semver-major. Large PRs are not getting landed quickly, it's highly likely that a clued-in reviewer will check in to add the label long before it's in a position to land, and we can always couple it with an action to conditionally comment a reminder when the PR is opened.

[zeexzeex]

You're right. I checked against the large-pr label: of the 6 PRs carrying it, my logic misses 2. #64429 is well under the threshold, and #62217 has a net negative diff plus the dependencies label, so it gets exempted twice over despite not being a routine update.

I'll rewrite this as a large-pr label check alongside the existing semver-major branch, and drop the line counting, the exemption list, and the additions/deletions query fields.

[zeexzeex]

Node.js requires two TSC approvals for large pull requests, but nothing enforces it at land time. This adds that check to git node land.

Whether a pull request is large is not something that can be derived from the diff. The policy counts a new subsystem as large regardless of size, and exempts routine dependency updates but not other dependency changes; neither is visible in the pull request data. An earlier revision of this PR tried a 5000 line threshold, and checking it against the 6 pull requests currently carrying the large-pr label, it missed 2 of them: nodejs/node#64429 is well under the threshold, and nodejs/node#62217 has a net negative diff.

The project already records this judgement with the large-pr label, so this checks that instead, the way semver-major is checked. The approval requirement is the same for both, so the existing branch is widened rather than duplicated, with a distinct message and reason code so a failure tells the author which rule applies. A pull request carrying both labels is reported as semver-major.

Refs: https://github.com/nodejs/node/blob/main/doc/contributing/large-pull-requests.md
Fixes: #1063

[Renegade334]

I don't think that trying to automatically detect whether a PR falls into the policy definition of a large PR is ever going to be feasible. This can't detect whether a PR adds a new subsystem, or detect whether a non-automated dependency change PR meets the large PR threshold (it's only automated commits that are exempt), and indeed a PR only needs to touch a single gypfile in /deps for this logic to exempt it.

I think we would be much better off making this a label-based check, à la fast track or semver-major. Large PRs are not getting landed quickly, it's highly likely that a clued-in reviewer will check in to add the label long before it's in a position to land, and we can always couple it with an action to conditionally comment a reminder when the PR is opened.

[zeexzeex]

You're right. I checked against the large-pr label: of the 6 PRs carrying it, my logic misses 2. #64429 is well under the threshold, and #62217 has a net negative diff plus the dependencies label, so it gets exempted twice over despite not being a routine update.

I'll rewrite this as a large-pr label check alongside the existing semver-major branch, and drop the line counting, the exemption list, and the additions/deletions query fields.

[zeexzeex]

Node.js requires two TSC approvals for large pull requests, but nothing enforces it at land time. This adds that check to git node land.

Whether a pull request is large is not something that can be derived from the diff. The policy counts a new subsystem as large regardless of size, and exempts routine dependency updates but not other dependency changes; neither is visible in the pull request data. An earlier revision of this PR tried a 5000 line threshold, and checking it against the 6 pull requests currently carrying the large-pr label, it missed 2 of them: nodejs/node#64429 is well under the threshold, and nodejs/node#62217 has a net negative diff.

The project already records this judgement with the large-pr label, so this checks that instead, the way semver-major is checked. The approval requirement is the same for both, so the existing branch is widened rather than duplicated, with a distinct message and reason code so a failure tells the author which rule applies. A pull request carrying both labels is reported as semver-major.

Refs: https://github.com/nodejs/node/blob/main/doc/contributing/large-pull-requests.md
Fixes: #1063

[Renegade334]

I don't think that trying to automatically detect whether a PR falls into the policy definition of a large PR is ever going to be feasible. This can't detect whether a PR adds a new subsystem, or detect whether a non-automated dependency change PR meets the large PR threshold (it's only automated commits that are exempt), and indeed a PR only needs to touch a single gypfile in /deps for this logic to exempt it.

I think we would be much better off making this a label-based check, à la fast track or semver-major. Large PRs are not getting landed quickly, it's highly likely that a clued-in reviewer will check in to add the label long before it's in a position to land, and we can always couple it with an action to conditionally comment a reminder when the PR is opened.

[zeexzeex]

You're right. I checked against the large-pr label: of the 6 PRs carrying it, my logic misses 2. #64429 is well under the threshold, and #62217 has a net negative diff plus the dependencies label, so it gets exempted twice over despite not being a routine update.

I'll rewrite this as a large-pr label check alongside the existing semver-major branch, and drop the line counting, the exemption list, and the additions/deletions query fields.

[zeexzeex]

Node.js requires two TSC approvals for large pull requests, but nothing enforces it at land time. This adds that check to git node land.

Whether a pull request is large is not something that can be derived from the diff. The policy counts a new subsystem as large regardless of size, and exempts routine dependency updates but not other dependency changes; neither is visible in the pull request data. An earlier revision of this PR tried a 5000 line threshold, and checking it against the 6 pull requests currently carrying the large-pr label, it missed 2 of them: nodejs/node#64429 is well under the threshold, and nodejs/node#62217 has a net negative diff.

The project already records this judgement with the large-pr label, so this checks that instead, the way semver-major is checked. The approval requirement is the same for both, so the existing branch is widened rather than duplicated, with a distinct message and reason code so a failure tells the author which rule applies. A pull request carrying both labels is reported as semver-major.

Refs: https://github.com/nodejs/node/blob/main/doc/contributing/large-pull-requests.md
Fixes: #1063

[Renegade334]

I don't think that trying to automatically detect whether a PR falls into the policy definition of a large PR is ever going to be feasible. This can't detect whether a PR adds a new subsystem, or detect whether a non-automated dependency change PR meets the large PR threshold (it's only automated commits that are exempt), and indeed a PR only needs to touch a single gypfile in /deps for this logic to exempt it.

I think we would be much better off making this a label-based check, à la fast track or semver-major. Large PRs are not getting landed quickly, it's highly likely that a clued-in reviewer will check in to add the label long before it's in a position to land, and we can always couple it with an action to conditionally comment a reminder when the PR is opened.

[zeexzeex]

You're right. I checked against the large-pr label: of the 6 PRs carrying it, my logic misses 2. #64429 is well under the threshold, and #62217 has a net negative diff plus the dependencies label, so it gets exempted twice over despite not being a routine update.

I'll rewrite this as a large-pr label check alongside the existing semver-major branch, and drop the line counting, the exemption list, and the additions/deletions query fields.