Skip to content

PSA: Label ready #17124

Description

@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.

Activity

  1. added
    metaIssues and PRs related to the general management of the project.
    author readyPRs with CI started, the required approvals, and no outstanding review comments.
    on Nov 18, 2017
  2. addaleax commented on Nov 18, 2017

    @addaleax
    MemberAuthor

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

  3. removed
    author readyPRs with CI started, the required approvals, and no outstanding review comments.
    on Nov 18, 2017
  4. gireeshpunathil commented on Nov 19, 2017

    @gireeshpunathil
    Member

    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?

  5. joyeecheung commented on Nov 19, 2017

    @joyeecheung
    Member

    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.

  6. joyeecheung commented on Nov 19, 2017

    @joyeecheung
    Member

    ^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.

  7. addaleax commented on Nov 19, 2017

    @addaleax
    MemberAuthor

    @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…

  8. tniessen commented on Nov 19, 2017

    @tniessen
    Member

    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.

  9. joyeecheung commented on Nov 19, 2017

    @joyeecheung
    Member

    @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?

  10. addaleax commented on Nov 19, 2017

    @addaleax
    MemberAuthor

    @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

  11. joyeecheung commented on Nov 19, 2017

    @joyeecheung
    Member

    @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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    metaIssues and PRs related to the general management of the project.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions