Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add needs-triage automation on k/k issues #11818

Closed
wants to merge 3 commits into from

Conversation

nikopen
Copy link
Contributor

@nikopen nikopen commented Mar 18, 2019

Following the discussion on https://groups.google.com/forum/#!topic/kubernetes-sig-contribex/BvGmOQ0v5f0 , a new label is proposed:

needs-sig-triage or similar.

Purpose of which is the following desired workflow:

                 |
            open bug/PR
                 |
                 V
        WAITING-ROOM: needs-sig, needs-sig-triage
                 |         ^
            (assign SIG)   |
                 |         |
                 V         |
      --> TRIAGE: needs-sig-triage<----
     |       /           \          |
     |  (close with   (verify)      |
     |   reason)          |         |
     |      |             V         |
      -- CLOSED      BACKLOG: kind/*, priority/*
                          |
                      (assign or claim)
                          |
                          V
                       IN-PROGRESS: assignee

i.e. - incoming new issue gets this label automatically, which signifies that it needs to be reviewed by the relevant SIG. After reviewing, it gets appropriate labels, is prioritized, or deleted if it's invalid / support request etc.

interrelated issue on how this will ultimately work - automatically triggered/de-triggered in kubernetes/community#3455

Open to suggestions on a different label and and overall comments.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. area/prow Issues or PRs related to prow sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Mar 18, 2019
@nikopen
Copy link
Contributor Author

nikopen commented Mar 18, 2019

@nikopen
Copy link
Contributor Author

nikopen commented Mar 18, 2019

/sig release
/sig pm
/kind design

@k8s-ci-robot k8s-ci-robot added sig/release Categorizes an issue or PR as relevant to SIG Release. sig/pm kind/design Categorizes issue or PR as related to design. labels Mar 18, 2019
Copy link
Member

@thockin thockin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!. I will start using it immediately, even before any support for auto-assignment :)

prow/plugins.yaml Outdated Show resolved Hide resolved
missing_comment: |
Awaiting SIG review/triage.

If it's a valid issue, please assign the relevant tracking labels (kind,priority etc) and remove `needs-review`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this configuration does what you want based on the state machine diagram, but I'm also a bit confused by the terminology. My understanding is that this label is meant to be applied to PRs in the triage state in the diagram, but this will actually be applied to PRs in the waiting room as well. This could be avoided by changing the regexp to ^triage/|^needs-sig$.

This message also suggests removing the needs-review label, but doesn't say how. Labels shouldn't be manually added or removed by humans, especially this label since the automation will just replace the label if a human tries to remove it.

Copy link
Contributor Author

@nikopen nikopen Mar 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, changed the label, regex and made the diagram clearer.

You're right about automatically removing the 'needs-sig-triage' label, and I wonder what the trigger would be. Perhaps by applying a 'lifecycle' label as in here: kubernetes/community#3455? That should be decided for the bigger picture

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind if the label is applied in the waiting room, it's equally true. That said, having been sig-triaged without a sig is a weird state to be left in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we're thinking of utilizing lifecycle labels as status indicators (lifecycle/ready to be worked on - lifecycle/active in progress - forgottenfrozen - stale etc), with exit criteria of needs-triage to be
regexp: ^(lifecycle)/(ready|active|frozen)$

@nikopen nikopen changed the title Add needs-review automation on k/k issues Add needs-sig-triage automation on k/k issues Mar 18, 2019
@nikhita
Copy link
Member

nikhita commented Mar 18, 2019

/hold

This needs to be broadly communicated: https://github.com/kubernetes/community/blob/master/sig-contributor-experience/charter.md#cross-cutting-and-externally-facing-processes

Will take a look tomorrow (my timezone). also cc @kubernetes/sig-contributor-experience-pr-reviews

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. sig/contributor-experience Categorizes an issue or PR as relevant to SIG Contributor Experience. labels Mar 18, 2019
@thockin
Copy link
Member

thockin commented Mar 27, 2019

Status on this?

@cblecker
Copy link
Member

cblecker commented Jun 3, 2019

/hold
Explicit hold, do not remove.

  • I don't think we should proceed with this during 1.15 freeze.
  • We still need to come to consensus on the larger triage workflow, and that hasn't happened. The right forum for that is contribex.
  • I suggest we add it to the agenda for an upcoming meeting, and discuss in detail the overall plan. Once we have a plan, then we can flip the switch on the new workflow right at the beginning of the 1.16 cycle.
  • From a technical perspective, new lifecycle labels need to be added to the plugin to be able to be applied:
    LifecycleActive = "lifecycle/active"
    LifecycleFrozen = "lifecycle/frozen"
    LifecycleRotten = "lifecycle/rotten"
    LifecycleStale = "lifecycle/stale"

How does that sound?

@nikopen
Copy link
Contributor Author

nikopen commented Jun 4, 2019

@spiffxp I believe that since this feature solves a problem where currently there's no solution at all, and people are explicitly requesting for it (SIG leads in particular) it can't be called an experiment.

re: stale / fallback every 90 days, I have already suggested upping the interval to 120 or more so this happens less frequently.
However still,

  1. if it indeed happens, then it means that the issue needs to be reviewed again and either put the frozen label or re-prioritize it. That's the sole purpose of the staleness feature of fejta-bot
  2. the intent is that SIGs will quickly reap the benefits of the new incoming features in order to (ideally!!) achieve the following:
  • better visibility over open issues
  • easier optimization of current work capacity
  • easier pre-planning over release cycles
  • better awareness of issues' current status
  • better outward visibility to external 'stakeholders' e.g. release team

and ultimately, less toil over issue triage and further management.

@nikopen
Copy link
Contributor Author

nikopen commented Jun 4, 2019

@cblecker

  • I think it can be enabled at any point in time given prior announcement and docs, it's not disruptive (needs-priority is often simply ignored) but perhaps people are very busy with freeze at this time.

  • Consensus is desired, but avoiding excessive bikeshedding is also desired - there should be a middle ground and constructive comments to move this (and more) forward.

  • re: meeting, it makes sense, though not sure if I'm able to attend this Wednesday evening, perhaps the next one.

  • re: technical, looks like it, you know these details more than me! Do let us know of the extra details needed.

cheers!

@cblecker
Copy link
Member

cblecker commented Jun 4, 2019

@nikopen Any time you're changing labels, automation, and workflows it's disruptive to contributors. As I've mentioned previously, I've been down this road before and received many complaints when we change things during important times in the cycle, or without excessive communication.

You're adding two extra labels to ~175 repos, and adding a comment requesting action being taken to 1550 open issues in k/k right now (https://github.com/kubernetes/kubernetes/issues?q=is%3Aopen+is%3Aissue+-label%3Alifecycle%2Factive+-label%3Alifecycle%2Ffrozen). This isn't an action to be taken lightly.

I think we already have some stuff on the agenda for this week's meeting (6/5) anyways, so maybe we want to pencil this in for discussion on our 6/12 meeting at 9:30am PT? Would that work for you?

@cblecker
Copy link
Member

cblecker commented Jun 4, 2019

To be clear: I'm very happy that you're spearheading this, @nikopen. I very much would like a triage workflow that we can adopt project-wide. I'm just wanting to convey that these are large scope actions anytime we're talking about project-wide, or even k/k-wide changes. They impact a lot of issues and a lot of contributors.

@nikopen
Copy link
Contributor Author

nikopen commented Jun 4, 2019

From that perspective indeed it seems like a bigger change! Happy that you're happy too.

Next week sounds good, let's speak soon

@cblecker
Copy link
Member

cblecker commented Jun 4, 2019

Added to our agenda 👍

@spiffxp
Copy link
Member

spiffxp commented Jun 5, 2019

@nikopen I am also happy you're spearheading this effort. I frame in terms of experiment to suggest that we define what our expected outcome is, try the thing, and adjust if the actual outcome isn't what we expected. You list a lot of great bullet points, I'm just asking... how will we know if those are actually happening?

re: my comment on lifecycle labels, I was trying to point out that the use of those lifecycle label implies re-triaging is necessary every 90 days, and it wasn't clear to me that's what the intended outcome of the discussion was I've seen this far. Perhaps we want to contrast that with a label that isn't auto-removed, to imply that triaged-once is good enough.

@justaugustus
Copy link
Member

/priority important-soon

@k8s-ci-robot k8s-ci-robot added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Jul 30, 2019
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 28, 2019
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Nov 27, 2019
@nikopen
Copy link
Contributor Author

nikopen commented Nov 28, 2019

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Nov 28, 2019
@justaugustus
Copy link
Member

@nikopen -- Any plans to pick this back up for 1.18?
/milestone v1.18

@justaugustus
Copy link
Member

Superseded by #16298, since this has stalled out.
/close

@k8s-ci-robot
Copy link
Contributor

@justaugustus: Closed this PR.

In response to this:

Superseded by #16298, since this has stalled out.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot
Copy link
Contributor

@nikopen: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/config Issues or PRs related to code in /config area/label_sync Issues or PRs related to code in /label_sync area/prow Issues or PRs related to prow cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/design Categorizes issue or PR as related to design. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/contributor-experience Categorizes an issue or PR as relevant to SIG Contributor Experience. sig/release Categorizes an issue or PR as relevant to SIG Release. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants