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

release: look for CLs with +2 but no Trust+1 #42176

Closed
ianlancetaylor opened this issue Oct 23, 2020 · 10 comments
Closed

release: look for CLs with +2 but no Trust+1 #42176

ianlancetaylor opened this issue Oct 23, 2020 · 10 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. recurring Issues that should never be closed, but moved to the next milestone once fixed in the current one. release-blocker
Milestone

Comments

@ianlancetaylor
Copy link
Contributor

As part of the transition to the release freeze, we should look for CLs that have a +2 but no Trust+1. Those CLs should all be reviewed for Trust+1 purposes, and, if no problems are found, submitted.

CC @dmitshur @cagedmantis @toothrot

@dmitshur
Copy link
Contributor

@toothrot has previously suggested this Gerrit search query:

https://go-review.googlesource.com/q/label:Code-Review%253D%252B2+label:Untrusted%253Dreject

There are 3.5~ pages of results (at 25 CL/page) at this time.

@dmitshur
Copy link
Contributor

dmitshur commented Oct 23, 2020

https://go-review.googlesource.com/q/label:Code-Review%253D%252B2+label:Untrusted%253Dreject+repo:go+branch:master+-label:Code-Review%253D-2+-hashtag:wait-author narrows it down to just the main repository, main branch, no -2 vote, no wait-author hashtag. There are 58 matching open CLs at time of posting.

@cagedmantis cagedmantis added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 23, 2020
@cagedmantis cagedmantis added this to the Go1.16 milestone Oct 23, 2020
@dmitshur
Copy link
Contributor

dmitshur commented Nov 4, 2020

Many CLs have been submitted since last time. I've looked over some of them and reviewed for Trust where it was the only missing thing. By now, the same query finds 15 CLs (down from 58). Many of those 15 have remaining work or need attention from someone, although maybe not all.

@toothrot toothrot self-assigned this Nov 5, 2020
@dmitshur
Copy link
Contributor

The query above (slightly modified to additionally filter out CLs that are waiting on author or have a -2 review) by now finds only 8 CLs. Of those, 6 are sent by people who have the ability to grant Trust+1 themselves. The other 2 were being actively evaluated for readiness for submission.

Given we're in the freeze and the week to submit in-flight changes has passed, I don't think there's more to do here.

@dmitshur dmitshur self-assigned this Nov 10, 2020
@cagedmantis
Copy link
Contributor

I was hoping that this issue would become a recurring issue that we kept open for each release.

@dmitshur
Copy link
Contributor

dmitshur commented Nov 10, 2020

@cagedmantis I was previously thinking this was needed for the Go 1.16 release only because the Trust requirement is new, and some CLs that could previously be submitted now needed an extra vote. But it does seem such a situation can continue to happen in the future if a CL is sent by someone without ability to self-Trust+1, and reviewed only with a single Code-Review+2. Thanks for pointing this out.

Maybe in the future this can be factored out to happen continuously as part of normal code review and not need to be tied as closely to the start of the release freeze, but I guess it'll work to make this issue recurring until that happens.

@dmitshur dmitshur reopened this Nov 10, 2020
@dmitshur dmitshur removed their assignment Nov 10, 2020
@dmitshur dmitshur added the recurring Issues that should never be closed, but moved to the next milestone once fixed in the current one. label Nov 10, 2020
@dmitshur dmitshur modified the milestones: Go1.16, Go1.17 Nov 10, 2020
@toothrot toothrot added this to Planned in Go Release Team Feb 18, 2021
@ianlancetaylor
Copy link
Contributor Author

Just a note that this is a steady ongoing problem that we ought to address somehow. It doesn't have anything to do with releases. We have a somewhat patchy and unreliable system for getting reviewers to look at CLs, but we don't have any system at all for getting CLs that have a +2 over the finish line to submission.

@kortschak
Copy link
Contributor

As a contributor that this is affecting I'd like to point out that it has a significant negative impact on the capacity of and motivation to contribute by non-blessed people.

@dmitshur
Copy link
Contributor

dmitshur commented Apr 2, 2021

Ian came up with an idea for us to add a weekly task for someone on the Go team (we have a weekly rotation we can make use of) to look for CLs that have been reviewed but just need trust to become submittable, and try to review them for trust.

https://go-review.googlesource.com/q/label:Code-Review%253D%252B2+label:Untrusted%253Dreject+-label:Code-Review%253D-2+-hashtag:wait-author+-age:1w finds CLs with CR+2 but no trust (without CR-2 or wait-author hashtag) that have been updated within the last 7 days. Perhaps it's a good place to look for the purpose of a weekly task. CC @toothrot.

@toothrot
Copy link
Contributor

We're actively looking for these issues as a weekly task. We can close this unless it becomes an issue again.

Go Release Team automation moved this from Planned to Done Apr 22, 2021
@golang golang locked and limited conversation to collaborators Apr 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. recurring Issues that should never be closed, but moved to the next milestone once fixed in the current one. release-blocker
Projects
Archived in project
Development

No branches or pull requests

6 participants