doc: simplify author ready

[BridgeAR]

The label should be applied early on. Otherwise there is little
benefit using this label at all.

The change to require two LGs does not imply that this is also required
for the author ready label.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[nodejs-github-bot]

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

[vsemozhetbyt]

Should we cc @nodejs/collaborators to PSA the change?

[joyeecheung]

Question: I have never really understood why the label is called author ready. I can understand what it means after reading the docs, but only after reading the docs...what's the meaning behind author in this context?

(This question does not have much thing to do with this PR itself, it's just a question)

[joyeecheung]

Personally this is how my brain interprets the label: imagine there is a queue where there are a bunch of "something" that work through it, check if the PRs in the queue have a green CI and pass the various checks enforced by git-node-land & core-validate-commit, and land it when the PR pass the checks. The label essentially puts the PR in that queue.

(At the moment "something" = nice collaborators who have some time to do the work of landing things, the ideal would be "something" = a bot but that would be a bit far away)

[addaleax]

@joyeecheung The author bit was added after the label was created, since it confused some people; it is supposed to say that there are no more tasks left for the author of the PR (unless maybe they are also a collaborator).

[Trott]

Landed in 4aabd7e

[joyeecheung]

The author bit was added after the label was created, since it confused some people; it is supposed to say that there are no more tasks left for the author of the PR (unless maybe they are also a collaborator).

The current description of the label does not seem to map to this accurately. If during the 7 day wait, another collaborator comes in and requests changes, then there are still tasks left for the author of the PR - to resolve the requested changes, in one way or another. And if the PR is semver-major, at least two TSC approvals are needed, in the worst case, a vote may be called and the decision may be against the current approach in the PR so the author may still need to update it if they still plan to pursue it, or they may have to close it.

[BridgeAR]

@joyeecheung if another author requests further changes, the label should be removed again. Attaching it once does not mean it stays there :)

[joyeecheung]

@BridgeAR Then what's the benefit of this label? For people landing PRs, they still need to look at the dates and count the reviews before go ahead and try landing the PR after seeing this label. For PR authors, they are still likely to wait and follow up with reviews after seeing this label. I think the label would be more useful if it's only applied when the criteria for reviews are satisfied and the only thing matters is to have someone coming in, making sure the CI is happy, checking the dates, and landing it - there is still a slight chance that someone may request changes before the PR gets landed by that would be less likely.

[joyeecheung]

In my understanding, the ideal state for a PR to land, in terms of code reviews, is:

1. It does not receive change requests in a sufficient amount of time, which signals that there is no obvious bugs in the code, or obvious mistakes in terms of decision making
2. It has as many as possible reviews from collaborators with sufficient expertise in the code being touched

Having only one approval is fine when we simply cannot get more people to approve it and there is no point stalling it since no one raises objection either, but that's not ideal, and does not imply that the PR itself is necessarily ready in terms of code quality. To me, having a label with the word ready applied when only one approval is available signals that either:

1. Only one approval is enough and the other approvals are simply formality, but that's not a good thing - we should not have such formality in the first place.
2. Or, we are not actually respecting the standard that we are pursuing, then we should either change the standard or change the label.