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

x/build: require TryBot-Result+1 or a new Ignore-TryBot label to submit CLs #56031

Closed
heschi opened this issue Oct 4, 2022 · 3 comments
Closed
Assignees
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@heschi
Copy link
Contributor

heschi commented Oct 4, 2022

Once TryBots have failed on a CL, the open comment thread serves to remind people to make them pass before submitting. However, nothing reminds authors/reviewers to run them in the first place. This is particularly annoying if you need a second Googler +1 vote, since https://go.dev/s/needs-review requires a passing TryBot run.

We can remind people that they need to run TryBots, and make it less likely that people will accidentally bypass them, by requiring an explicit vote from either an approver or the TryBots themselves for submission.

Specifically, we would add a submit requirement like label:TryBot-Result=+1 OR label:TryBot-Bypass=+1. The name of the new label is up for bikeshedding.

cc @golang/release

@heschi heschi added Builders x/build issues (builders, bots, dashboards) NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Oct 4, 2022
@heschi heschi added this to the Unreleased milestone Oct 4, 2022
@heschi
Copy link
Contributor Author

heschi commented Oct 26, 2022

@rsc suggests we could look into blocking on unresolved comments as well.

@heschi heschi self-assigned this Oct 26, 2022
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Oct 27, 2022
@heschi
Copy link
Contributor Author

heschi commented Nov 2, 2022

Done.

@heschi heschi closed this as completed Nov 2, 2022
@gopherbot
Copy link

Change https://go.dev/cl/452135 mentions this issue: cmd/gopherbot: rely on Gerrit config for TryBot and resolved comments

gopherbot pushed a commit to golang/build that referenced this issue Nov 30, 2022
As of go.dev/issue/56031, the need to have passing TryBots and no
unresolved comment threads is enforced by a submit requirement in
Gerrit. The code in the auto-submit task is no longer load-bearing,
so remove it to simplify and to avoid creating a false impression of
it doing something important (beyond little differences, like handling
the TryBot-Bypass label as implemented by the Gerrit submit rule, etc.).

Running the "auto-submit CLs" task in dry-run mode did not report
that it would submit any CLs.

For golang/go#48021.
Updates golang/go#56031.

Change-Id: I09c6900199d26bd8e90fe7f7f681fd6227a762e8
Reviewed-on: https://go-review.googlesource.com/c/build/+/452135
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
@golang golang locked and limited conversation to collaborators Nov 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
Archived in project
Development

No branches or pull requests

3 participants