Skip to content

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

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
Open
bradfitz opened this issue Apr 13, 2018 · 22 comments
Open

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

bradfitz opened this issue Apr 13, 2018 · 22 comments
Labels
Builders x/build issues (builders, bots, dashboards) Documentation Issues describing a change to documentation. NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@bradfitz
Copy link
Contributor

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 [email protected], 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.
@bradfitz bradfitz added Documentation Issues describing a change to documentation. NeedsFix The path to resolution is known, but the work has not been done. labels Apr 13, 2018
@bradfitz bradfitz self-assigned this Apr 13, 2018
@gopherbot gopherbot added this to the Unreleased milestone Apr 13, 2018
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Apr 13, 2018
@gopherbot
Copy link
Contributor

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

@FiloSottile
Copy link
Contributor

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

@bradfitz
Copy link
Contributor Author

@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 pushed a commit to golang/build that referenced this issue Apr 13, 2018
@bradfitz
gerrit: add support for hashtags

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
a9c7bdc 
Tested in one-off local tool:

   func main() {
       gc := gerrit.NewClient("https://go-review.googlesource.com/", gerrit.GitCookiesAuth())
       ....

Updates golang/go#24836

Change-Id: I231e3afb4a27e41f9b56968e3e97fa1c31fd8d84
Reviewed-on: https://go-review.googlesource.com/106795
Reviewed-by: Andrew Bonventre <[email protected]>
@gopherbot
Copy link
Contributor

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

gopherbot pushed a commit to golang/build that referenced this issue Apr 16, 2018
@bradfitz
maintner: add hashtag mutation accessors on GerritMeta
0b261d0 
Updates golang/go#24836

Change-Id: Id529f12dba54366d49feb112662473ca214ba2f6
Reviewed-on: https://go-review.googlesource.com/107297
Reviewed-by: Chris Broadfoot <[email protected]>
@gopherbot
Copy link
Contributor

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

@bcmills
Copy link
Contributor

bcmills commented Apr 16, 2018

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

@bradfitz
Copy link
Contributor Author

@bcmills, left bar of the PolyGerrit UI:
screen shot 2018-04-16 at 3 56 12 pm

@bradfitz
Copy link
Contributor Author

Or for the old UI, that little icon on the right:
screen shot 2018-04-16 at 3 57 11 pm

gopherbot pushed a commit to golang/build that referenced this issue Apr 17, 2018
@bradfitz
maintner: track hashtag edits on GerritMeta, make HashtagEdits method…
dd35151 
… faster

Updates golang/go#24836

Change-Id: I75cae7de574af7525964bdf420328d3e553a044c
Reviewed-on: https://go-review.googlesource.com/107305
Reviewed-by: Andrew Bonventre <[email protected]>
@bcmills
Copy link
Contributor

bcmills commented Apr 17, 2018

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
Copy link
Contributor Author

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

@bradfitz
Copy link
Contributor Author

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
Copy link
Contributor Author

@bcmills, submitted.

@gopherbot
Copy link
Contributor

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

@gopherbot
Copy link
Contributor

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

gopherbot pushed a commit to golang/build that referenced this issue Apr 20, 2018
@bradfitz
cmd/gopherbot: remove the wait-author hashtags from CLs when author r…
cbf9937 
…eplies

Updates golang/go#24836

Change-Id: I65dd57290634b31b112062dca9fafa76b2cc7153
Reviewed-on: https://go-review.googlesource.com/108218
Reviewed-by: Andrew Bonventre <[email protected]>
gopherbot pushed a commit to golang/build that referenced this issue Apr 23, 2018
@bradfitz
devapp: hide CLs with hashtag wait-author
6e90891 
Updates golang/go#24836

Change-Id: I9964dc7ec2de21201b9258709803623482aa4104
Reviewed-on: https://go-review.googlesource.com/108219
Reviewed-by: Andrew Bonventre <[email protected]>
@gopherbot
Copy link
Contributor

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

gopherbot pushed a commit to golang/build that referenced this issue May 17, 2018
@bradfitz
devapp: also hide open reviews with the tag "wait-release"
1d5048e 
Updates golang/go#24836

Change-Id: I7b739a54dec374d60e66bf083cd41c1879500ed3
Reviewed-on: https://go-review.googlesource.com/113536
Reviewed-by: Bryan C. Mills <[email protected]>
@ALTree
Copy link
Member

ALTree commented Aug 2, 2018

@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.

defer added a commit to defer/gerrit-events that referenced this issue Oct 9, 2018
@defer
Add hashtags-changed event
149b26f 
With the arrival of NoteDB[1], gerrit now supports hashtags on changes
which might open interesting use-cases, like go's use of hashtags for
managing CL[2].

This patch adds support for the hashtag changed event.

[1] https://gerrit-review.googlesource.com/Documentation/note-db.html
[2] golang/go#24836
@defer defer mentioned this issue Oct 9, 2018
Merged
defer added a commit to defer/gerrit-events that referenced this issue Oct 9, 2018
@defer
Add hashtags-changed event
7a40b1b 
With the arrival of NoteDB[1], gerrit now supports hashtags on changes
which might open interesting use-cases, like go's use of hashtags for
managing CL[2].

This patch adds support for the hashtag changed event.

[1] https://gerrit-review.googlesource.com/Documentation/note-db.html
[2] golang/go#24836
@gopherbot
Copy link
Contributor

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

@bcmills
Copy link
Contributor

bcmills commented Feb 6, 2019

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

This indeed all needs documentation.

@bradfitz bradfitz removed their assignment May 29, 2019
@bradfitz bradfitz changed the title x/build: start using Gerrit hashtags for managing open CLs x/build: finish, document Gerrit hashtags for managing open CLs May 29, 2019
@dmitshur
Copy link
Member

dmitshur commented Jan 24, 2020 •
edited
Loading

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
Copy link
Contributor

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

gopherbot pushed a commit to golang/build that referenced this issue Mar 11, 2022
@rolandshoemaker @gopherbot
cmd/gopherbot: don't autosubmit CLs with wait-release
aa7fa4b 
If a CL has the "wait-release" hashtag set, don't autosubmit it, since
we are presumably in the freeze, and should wait for the tree to open.

Updates golang/go#48021
Updates golang/go#24836

Change-Id: I23258ccbb71d2b8febe97ad8883b9fe9f90c761d
Reviewed-on: https://go-review.googlesource.com/c/build/+/391734
Reviewed-by: Dmitri Shuralyov <[email protected]>
Trust: Roland Shoemaker <[email protected]>
Run-TryBot: Roland Shoemaker <[email protected]>
Auto-Submit: Roland Shoemaker <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
@cagedmantis cagedmantis moved this to Planned in Go Release Jun 27, 2023
@dmitshur dmitshur mentioned this issue Apr 26, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Builders x/build issues (builders, bots, dashboards) Documentation Issues describing a change to documentation. NeedsFix The path to resolution is known, but the work has not been done.
Projects
Go Release
Status: Planned
Milestone
Unreleased
Development

No branches or pull requests
6 participants
@bradfitz @FiloSottile @dmitshur @ALTree @bcmills @gopherbot