Node: PSA: Label `ready`

[addaleax]

@nodejs/collaborators

I’ve added the label ready for PRs where I’ve just kicked off CI and that are good to go (not outstanding review items), pending the CI outcome and possibly the 48/72 hour waiting rule.

I think it’s going to help me – and so it might also help others – keep track of what’s ready/not; If this doesn’t work out, we can always just remove that.

[addaleax]

Okay, I’m done now. If you think this is a horrible idea, please yell loudly at me.

[gireeshpunathil]

this is a great idea in:

• saving manual efforts in scavenging through open PRs to figure out the status and pending action
• saving infra cost where CI results got recycled went un-noticed and needing to run again
• improving the review-CI-landing cycle latency

However, this label looks transient by nature (holding its stated meaning within a subset of the PR cycle) - how do we deal with it? add the label when the PR is ready and remove it when the PR lands?

[joyeecheung]

Love this idea, though I would suggest to rename it to ready-after-ci or something that implies "it's pending a green CI", just so that new contributors would not get confused.

BTW, do we have a label for PRs that are not ready? I thought we have do-not-land but apparently there isn't.

[joyeecheung]

^Oh, there is also the wait rule...maybe wait-for-ci & wait-for-48/72h? We can label the PR both or one of them, and remove the label when the requirements are met (we can also teach the bot to put and remove the wait-for-48/72h label. for the wait-for-ci label I would be careful and rather let a human to decide at the moment though). It would help new contributors understand what is holding the PR.

[addaleax]

@gireeshpunathil @Trott suggested that we remove the label when landing so that it doesn’t show up in issue searches where people don’t add is:open, I think that’s the only difference.

@joyeecheung The thing is, both whether CI finished and the time since the PR was opened are visible in the Github interface anyway…

[tniessen]

remove the label when landing so that it doesn’t show up in issue searches

I guess this could be added to https://github.com/nodejs/github-bot in case people don't notice the label.

[joyeecheung]

@addaleax

The thing is, both whether CI finished and the time since the PR was opened are visible in the Github interface anyway…

Oh right...those labels would not imply that "this PR has been approved"..:/ Anyway I think this new label is worth documenting in the docs?

[addaleax]

@joyeecheung Where in the docs? Also, I’d probably want to wait a few days and see how the workflow evolves around it, if it does at all.

edit: also, my intention was to only add this to PRs that don’t need more approvals

[joyeecheung]

@addaleax Yeah we can wait and see how it goes before documenting it.

I am thinking the collaborator guide, probably somewhere in https://github.com/nodejs/node/blob/master/COLLABORATOR_GUIDE.md#code-reviews-and-consensus-seeking