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.

Sign up for GitHub

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

Jump to bottom

x/build/cmd/gopherbot: remove NeedsDecision on proposals #31788

Closed
bradfitz opened this issue May 1, 2019 · 12 comments
Closed

x/build/cmd/gopherbot: remove NeedsDecision on proposals #31788

bradfitz opened this issue May 1, 2019 · 12 comments
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@bradfitz
Copy link
Contributor

bradfitz commented May 1, 2019

Per @golang/proposal-review, the bot should remove NeedsDecision on proposals. It's redundant and messes with searches.
@gopherbot gopherbot added this to the Unreleased milestone May 1, 2019
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label May 1, 2019
@bcmills
Copy link
Contributor

bcmills commented May 20, 2019

The Needs labels are added by humans. The Proposal label is added by bots.

The NeedsDecision label indicates that a proposal has been triaged by humans, which is a useful signal for whether the proposal is ready for a more in-depth review by the other humans who own the package(s) in question.

@bcmills
Copy link
Contributor

bcmills commented May 20, 2019

...unless the idea is for the proposal committee to be the ones to loop in the humans? But that seems like it adds needless overhead for the proposal committee and needless delay in getting feedback from owners.

@bcmills bcmills added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label May 20, 2019
@ianlancetaylor
Copy link
Member

Every proposal needs a decision. It doesn't seem to help to add the NeedsDecision label to a proposal.

You suggest a possible meaning for NeedsDecision on proposals, but in fact the proposal review team doesn't use that label, and doesn't have a need for it.

(For Go 2 proposals, on the other hand, we do use NeedsDecision, to mean that the proposal seems ready and we just need to decide whether or not to adopt it.)

@bcmills
Copy link
Contributor

bcmills commented May 21, 2019

The NeedsDecision label is more of a cue for folks triaging issues than for the proposal committee: some proposals clearly need more information, or clearly need feedback from specific owners, or are duplicates of existing proposals, so it's useful for folks to have some signal about whether that first-pass check has been completed.

The Proposal label does not capture that first-pass check, since Gopherbot applies it automatically based on the issue title.

Currently, the NeedsDecision label does, since it is not automatic.

As an alternative, I suppose we could change Goptherbot to stop adding the Proposal label. Then that label itself could serve as an indicator of triage status.

@ianlancetaylor
Copy link
Member

Sounds like we are using NeedsDecision for two different unrelated purposes.

@rsc
Copy link
Contributor

rsc commented May 28, 2019

The triage rotation does not need to triage Proposal issues. It would help the proposal reviewers if the proposals did not say NeedsDecision. @bradfitz says the triage github search shortcut says -label:proposal anyway.

Can we agree that Proposals can drop the NeedsDecision label automatically?

@dmitshur
Copy link
Contributor

/cc @julieqiu FYI, since you were considering changes to the triage search link.

@bcmills
Copy link
Contributor

bcmills commented May 29, 2019

If y'all really don't want any first-level triage on proposals, then I guess it's fine to drop the NeedsDecision label from those issues.

@rsc
Copy link
Contributor

rsc commented Jun 4, 2019

Yes, we don't need any first-level triage on proposals. Thanks.

@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 4, 2019
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jun 4, 2019
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/180925 mentions this issue: cmd/gopherbot: drop NeedsDecision when add Proposal

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/183624 mentions this issue: cmd/gopherbot: don't fight over NeedsDecision removal

gopherbot pushed a commit to golang/build that referenced this issue Jun 24, 2019
@dmitshur
cmd/gopherbot: don't fight over NeedsDecision removal
b8243eb 
This is a followup to CL 180925 to prevent gopherbot from fighting
people (too much) if they decide to override gopherbot's behavior
and re-add the NeedsDecision label. It's done by adding a check for
whether gopherbot has already taken the action and avoid repeating
it if so.

Since this action is removing a label, we can't just check for any
"labeled" event, as those are likely to happen for other reasons.
So add a more precise check for whether gopherbot has previously
removed the "NeedsDecision" label from the target issue.

Updates golang/go#31788
Updates golang/go#21312

Change-Id: Iaf4dd69a5bfd637694995ee60869f94362110a7d
Reviewed-on: https://go-review.googlesource.com/c/build/+/183624
Reviewed-by: Andrew Bonventre <andybons@golang.org>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/193120 mentions this issue: cmd/gopherbot: don't remove NeedsDecision from Go 2 issues

gopherbot pushed a commit to golang/build that referenced this issue Sep 4, 2019
@bradfitz
cmd/gopherbot: don't remove NeedsDecision from Go 2 issues
54acf3a 
CL 180925 was a bit too aggressive. We've since cleaned up its changes
(see patchset 1 of this CL).

Updates golang/go#31788

Change-Id: I96b8fa4f5cbc158869e4f607aa69be130c0eda75
Reviewed-on: https://go-review.googlesource.com/c/build/+/193120
Reviewed-by: Ian Lance Taylor <iant@golang.org>
codebien pushed a commit to codebien/build that referenced this issue Nov 13, 2019
@letientai299 @codebien
cmd/gopherbot: remove NeedsDecision label on proposals

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Learn about vigilant mode
6d2ed33 
Fixes golang/go#31788

Change-Id: I775f706a154f3584c24b1bd2f4925cbc293f8740
Reviewed-on: https://go-review.googlesource.com/c/build/+/180925
Reviewed-by: Andrew Bonventre <andybons@golang.org>
codebien pushed a commit to codebien/build that referenced this issue Nov 13, 2019
@dmitshur @codebien
cmd/gopherbot: don't fight over NeedsDecision removal

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Learn about vigilant mode
4cf3e98 
This is a followup to CL 180925 to prevent gopherbot from fighting
people (too much) if they decide to override gopherbot's behavior
and re-add the NeedsDecision label. It's done by adding a check for
whether gopherbot has already taken the action and avoid repeating
it if so.

Since this action is removing a label, we can't just check for any
"labeled" event, as those are likely to happen for other reasons.
So add a more precise check for whether gopherbot has previously
removed the "NeedsDecision" label from the target issue.

Updates golang/go#31788
Updates golang/go#21312

Change-Id: Iaf4dd69a5bfd637694995ee60869f94362110a7d
Reviewed-on: https://go-review.googlesource.com/c/build/+/183624
Reviewed-by: Andrew Bonventre <andybons@golang.org>
codebien pushed a commit to codebien/build that referenced this issue Nov 13, 2019
@bradfitz @codebien
cmd/gopherbot: don't remove NeedsDecision from Go 2 issues

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Learn about vigilant mode
4453015 
CL 180925 was a bit too aggressive. We've since cleaned up its changes
(see patchset 1 of this CL).

Updates golang/go#31788

Change-Id: I96b8fa4f5cbc158869e4f607aa69be130c0eda75
Reviewed-on: https://go-review.googlesource.com/c/build/+/193120
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Sep 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
6 participants
@bradfitz @rsc @dmitshur @ianlancetaylor @bcmills @gopherbot