Repository navigation
PSA: Label ready #17124
Description
Activity
- addedmetaIssues and PRs related to the general management of the project.Issues and PRs related to the general management of the project.author readyPRs with CI started, the required approvals, and no outstanding review comments.PRs with CI started, the required approvals, and no outstanding review comments.
on Nov 18, 2017 Okay, I’m done now. If you think this is a horrible idea, please yell loudly at me.
- removedauthor readyPRs with CI started, the required approvals, and no outstanding review comments.PRs with CI started, the required approvals, and no outstanding review comments.
on Nov 18, 2017 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?
Love this idea, though I would suggest to rename it to
ready-after-cior 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.
^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 thewait-for-48/72hlabel. for thewait-for-cilabel 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.@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…
Reacted by Refael Ackermann and Gireesh Punathilremove 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.
Reacted by Benjamin GruenbaumThe 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?
@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
@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
@nodejs/collaborators
I’ve added the label
readyfor 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.