golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.96k stars 17.66k forks source link

x/build: finish, document Gerrit hashtags for managing open CLs #24836

Open bradfitz opened 6 years ago

bradfitz commented 6 years ago

In chat with @andybons @FiloSottile @bcmills and @ianlancetaylor yesterday, I proposed we start using Gerrit's "hashtags" support, now that our hosted Gerrit supports it (notedb support is active for us) and PolyGerrit (the new web UI) supports it.

After discussion, we settled on using tags:

so, how about: "wait-author", "wait-release", "wait-issue", "wait-$PERSON"
where $PERSON is some substring of a name/email in the reviewer set.
and if somebody is named "Issue Release" and their email is issue@release.com, then you have to use their full email in $PERSON
we can mass remove the "wait-release" labels when tree opens

While triaging today, I found we should also add support for:

wait-cl-NNNNN -- this CL is blocked until NNN is resolved

I started looking into adding hashtag support to maintner and gopherbot.

Tracking that work here, then documenting all this on the wiki.

gopherbot commented 6 years ago

Change https://golang.org/cl/106795 mentions this issue: gerrit: add support for hashtags

FiloSottile commented 6 years ago

Also wait-issue-NNNNN. I guess the number can be implicit when the issue is linked from the commit message.

bradfitz commented 6 years ago

@FiloSottile, yup, I just found that too and just tagged an issue with that, for a CL that had an open proposal but wasn't mentioned in the commit message.

gopherbot commented 6 years ago

Change https://golang.org/cl/107297 mentions this issue: maintner: add hashtag mutation accessors on GerritMeta

gopherbot commented 6 years ago

Change https://golang.org/cl/107305 mentions this issue: maintner: track hashtag edits on GerritMeta, make HashtagEdits method faster

bcmills commented 6 years ago

I think I must be missing something, but how do I add hashtags from the Gerrit UI?

bradfitz commented 6 years ago

@bcmills, left bar of the PolyGerrit UI:

screen shot 2018-04-16 at 3 56 12 pm
bradfitz commented 6 years ago

Or for the old UI, that little icon on the right:

screen shot 2018-04-16 at 3 57 11 pm
bcmills commented 6 years ago

I see the ADD HASHTAG button on my own changes, but not on other changes. Is that expected?

screenshot 2018-04-17 at 11 05 57

bradfitz commented 6 years ago

Yes. We haven't modified the default permissions yet to give our Approvers group edit access.

bradfitz commented 6 years ago

Sent change modifying our Gerrit config in https://go-review.googlesource.com/c/All-Projects/+/107556

[access "refs/heads/*"]
...
    editTopicName = group approvers
    label-Run-TryBot = +0..+1 group approvers
    label-Run-TryBot = +0..+1 group may-start-trybots
    label-TryBot-Result = -1..+1 group trybot-result-changers
    editHashtags = group approvers

... adding that last line.

bradfitz commented 6 years ago

@bcmills, submitted.

gopherbot commented 6 years ago

Change https://golang.org/cl/108218 mentions this issue: cmd/gopherbot: remove the wait-author hashtags from CLs when author replies

gopherbot commented 6 years ago

Change https://golang.org/cl/108219 mentions this issue: devapp: hide CLs with hashtag wait-author

gopherbot commented 6 years ago

Change https://golang.org/cl/113536 mentions this issue: devapp: also hide open reviews with the tag "wait-release"

ALTree commented 6 years ago

@bradfitz is there some note/doc explaining how these "Gerrit hashtags" are used in the Go project? I searched the wiki for "hashtag" but I couldn't find anything.

I've seen people tagging CLs (mostly with wait-release) but I still don't understand what exactly means to do that, what's the full list of the available hashtags, how hashtags are supposed to be used, how they integrate with the infrastructure and the dashboards, and whether wait-release supersedes the old R=go1.12 syntax.

gopherbot commented 5 years ago

Change https://golang.org/cl/158578 mentions this issue: devapp: hide CLs with +2 Core-Review, any "wait-*" hashtag

bcmills commented 5 years ago

I don't see any associated CLs to remove wait-issue tags.

I think we should remove wait-issue-NNNNN when issue NNNNN either is closed or receives a NeedsFix or Proposal-Accepted label.

bradfitz commented 5 years ago

I don't think we need or want wait-issue-NNNN. I think it should only be implicit. That also means I can add wait-issue on CLs that add code or API with an associated issue.

The wait-issue would only be removed once an issue is referenced and NeedsFix-ed or Proposal-Accepted.

bradfitz commented 5 years ago

@ALTree, yes, wait-release replaces the old R=go1.12 syntax.

This indeed all needs documentation.

dmitshur commented 4 years ago

In CL 216199, I've used wait-time-20200128 to indicate the CL is waiting for the day January 28, 2020 before it can be submitted.

Edit: Also in CL 223927.

gopherbot commented 2 years ago

Change https://go.dev/cl/391734 mentions this issue: cmd/gopherbot: don't autosubmit CLs with wait-release