cherry-pick bug policy

[Brad Fitzpatrick]

I thought we agreed on this previously, but we seem to have split into two camps again.

When cherry-picking stuff to a point release (e.g. 1.8.1), do we:

a) open a NEW bug for the cherry-pick, referencing the CL to cherry-pick and the old bug.

b) re-open and re-milestone the main bug (e.g. the Go1.9-milestoned bug) to Go1.8.1?

I prefer (a), and I thought we'd already agreed on (a).

Do people strongly prefer (b)?

[Chris Broadfoot]

I prefer (a) as well, though it does mean either editing the CL message to include the issue number or closing the bug manually.

(b) is fine, too, but means the milestone issue view ends up being less accurate.

[Austin Clements]

Was this convention communicated when it was decided on? I can't find anything in my email (which doesn't mean it's not there). I started doing (b) because it looked like what other people were doing (including Brad :).

In the process of doing (b) I found that I strongly dislike it. It interacts very poorly with Github's auto-closer, which closes the issue when it gets committed to master (which requires manual re-opening, which often doesn't happen) and does not re-close the issue when it gets committed to the release branch (which requires manually re-closing the issue, which often doesn't happen). This means you can't get any information whatsoever about the state of things by looking at issue search pages.

(a) seems fine. It requires a little more work, both to write the new issue and during cherry-picking, and requires that you remember to create the new issue once the CL is ready. But it's dramatically better information-wise.

Did we consider using Github labels for this? There seem to be three states to issues that are candidates for the release branch: "not fixed", "fixed on master, not cherry-picked", "fixed on master and cherry-picked to release". Could we have just one issue, which is milestoned for the release, let "closed" mean "fixed on master, not cherry-picked" (which the auto-closer will do for us), and use a label to mark "fixed on master and cherry-picked"? A bot could even apply this label when it sees the cherry-pick, though doing it by hand would be easy, too. This requires no more work than (b), keeps state transitions monotonic, and clearly shows the full state of the world directly in the search results.

[Ian Lance Taylor]

I also thought we had agreed on (a), but then I saw people doing (b).

I don't care either way, except to say that if we do (a) we need to
make sure to annotate the existing issue to point to the new one.

Ian

[Matthew Dempsky]

On Tue, Mar 21, 2017 at 1:52 PM, Ian Lance Taylor <ia...@golang.org> wrote:

I don't care either way, except to say that if we do (a) we need to

make sure to annotate the existing issue to point to the new one.

As long as we mention the old issue in the new one, GitHub will include a cross-reference for us.

My preference is also for A.

[Brad Fitzpatrick]

On Tue, Mar 21, 2017 at 1:52 PM, Ian Lance Taylor <ia...@golang.org> wrote:

Github will do that for us if there's a reference the other direction. So just make the new bug link to the old one and you get the back edge for free.

[Brad Fitzpatrick]

On Tue, Mar 21, 2017 at 1:30 PM, Austin Clements <aus...@google.com> wrote:

Was this convention communicated when it was decided on? I can't find anything in my email (which doesn't mean it's not there). I started doing (b) because it looked like what other people were doing (including Brad :).

It might've only been in a release meeting? Or some comment on Gerrit or Github? I can't find it in email either.

[Damian Gryski]

Should this be written down somewhere in a wiki page or release procedure steps?

Damian

[Chris Broadfoot]

Sounds like there's a slight preference for (a). I'll document the policy on the wiki.

On filing these bugs... I noticed there are a couple bugs with the title:

"Backport CL X to Go 1.8.1"

Let's also decide on a policy about the titles. They should be descriptive enough so that people linked to the milestone from release notes get an idea of what went into the minor release.

Most of the time, that's just going to be the original bug title.

[Brad Fitzpatrick]

The current titles are fine for now. I figure gopherbot can rewrite them soon enough.

--
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Austin Clements]

I'm still a bit confused by the expected procedure here.

I'm in the middle of fixing https://github.com/golang/go/issues/19724. This issue is currently milestoned for Go 1.8.1. Here's what I think I should do:

1. Fix it on master. (This will close the issue automatically. For the moment it looks like it's been fixed for 1.8.1.)

2. Create a new issue (possibly with the same title as the original issue?) and milestone it to Go 1.8.1 so people know it has not been fixed for 1.8.1. Make sure the description references the original issue so they get linked.

3. Re-milestone the original issue to 1.9 to get it out of the 1.8.1 issue list.

4. Cherry-pick the CL to the 1.8 release branch, remembering to change the "fixes #" to the new issue.

5. Once the new CL is submitted, remember to close the cherry-pick issue, since it won't get automatically closed.

Is this the right procedure? It seems rather manual and error-prone. (It still seems like a single issue and a label for "cherry-picked" would be much easier.)

[Brad Fitzpatrick]

On Tue, Mar 28, 2017 at 1:23 PM, Austin Clements <aus...@google.com> wrote:

I'm still a bit confused by the expected procedure here.

I'm in the middle of fixing https://github.com/golang/go/issues/19724. This issue is currently milestoned for Go 1.8.1. Here's what I think I should do:

1. Fix it on master. (This will close the issue automatically. For the moment it looks like it's been fixed for 1.8.1.)

You don't have to write "Fixes #nnn". You can write:

    Updates #nnnn (fixes, but needs cherry-pick to Go 1.8)

[Brad Fitzpatrick]

On Tue, Mar 28, 2017 at 1:23 PM, Austin Clements <aus...@google.com> wrote:

I'm still a bit confused by the expected procedure here.

I'm in the middle of fixing https://github.com/golang/go/issues/19724. This issue is currently milestoned for Go 1.8.1. Here's what I think I should do:

1. Fix it on master. (This will close the issue automatically. For the moment it looks like it's been fixed for 1.8.1.)

2. Create a new issue (possibly with the same title as the original issue?) and milestone it to Go 1.8.1 so people know it has not been fixed for 1.8.1. Make sure the description references the original issue so they get linked.

3. Re-milestone the original issue to 1.9 to get it out of the 1.8.1 issue list.

4. Cherry-pick the CL to the 1.8 release branch, remembering to change the "fixes #" to the new issue.

5. Once the new CL is submitted, remember to close the cherry-pick issue, since it won't get automatically closed.

Is this the right procedure? It seems rather manual and error-prone. (It still seems like a single issue and a label for "cherry-picked" would be much easier.)

Are you proposing we use both labels and milestones? Or just one?

We can't auto-label or auto-unlabel with commit messages.

I'd prefer to keep labels out of this.

[Austin Clements]

On Tue, Mar 28, 2017 at 4:25 PM, Brad Fitzpatrick <brad...@golang.org> wrote:

On Tue, Mar 28, 2017 at 1:23 PM, Austin Clements <aus...@google.com> wrote:

I'm still a bit confused by the expected procedure here.

I'm in the middle of fixing https://github.com/golang/go/issues/19724. This issue is currently milestoned for Go 1.8.1. Here's what I think I should do:

1. Fix it on master. (This will close the issue automatically. For the moment it looks like it's been fixed for 1.8.1.)

You don't have to write "Fixes #nnn". You can write:

    Updates #nnnn (fixes, but needs cherry-pick to Go 1.8)

I don't see how this helps. Now there's another step where I have to manually close the original issue, and I still have to update the issue number in the commit message when cherry-picking. I suppose it eliminates one hazard where the original issue temporarily winds up on the closed list for the milestone.

[Austin Clements]

I'm proposing we have a "Cherry-picked" label (just one; not per milestone or anything), with the following procedure:

0. Original issue is filed and milestoned for 1.8.1.

1. Fix it on master. Say "Fixes #NNN" just like you would for any other issue and let GitHub close it.

2. Cherry-pick the CL to the 1.8 release branch. Submit it.

3. Apply the "cherry-picked" label to the issue. (A robot could do this, but it's also the sole step that's not directly involved in getting the CL into the release.)

This way there's only one issue per issue. The issues that still need to be fixed are "open". The issues that have been fixed are "closed". And the issues that have been fixed but still need to be cherry-picked are under the right milestone and just missing the label. The only step you can possibly forget is 3, and there's little harm in that since someone will notice when they go to cherry-pick it again and apply the label at that point.

[Brad Fitzpatrick]

And if it's a security issue being cherry-picked to the past two releases?

[Austin Clements]

Then you apply the label once it's been cherry-picked to both. Do we currently create a GitHub milestone for the single issue that will be cherry-picked into that security release?

I own a lot of the issues that have to get cherry-picked. If I have to go through the process I outlined earlier (do I? no one has said that's indeed the process), then I am going to screw it up. I don't care what the process is. I just don't want to be the one to screw it up.

[Brad Fitzpatrick]

On Tue, Mar 28, 2017 at 2:23 PM, Austin Clements <aus...@google.com> wrote:

Then you apply the label once it's been cherry-picked to both. Do we currently create a GitHub milestone

Milestone?

 

for the single issue that will be cherry-picked into that security release?

If we had separate labels for CherryPickedToGo1.8 vs CherryPickedGo1.7, then we could automate the labeling with a bot at least.

 

I own a lot of the issues that have to get cherry-picked. If I have to go through the process I outlined earlier (do I? no one has said that's indeed the process), then I am going to screw it up. I don't care what the process is. I just don't want to be the one to screw it up.

I agree we need to figure out the process and use it consistently. Once it's documented we can also automate much of it hopefully.

[Russ Cox]

How about this. A bot watches issues in point release milestones. If such an issue is auto-closed due to a 'Fixes' commit on master, the bot says to itself "aha this hasn't been cherry-picked yet" and automatically reopens the issue with a 1-line comment "Reopening: CL xxxxx is not on release branch." Similarly, when the eventual cherry-pick does get committed to the release branch (which does not auto-close the issue), the bot comes along and closes the issue with another 1-line comment "Closing: CL xxxxx cherry-picked to release-branch as CL xxxxx." Bonus points for markdown links around [CL xxxxx].

Given this, when an issue comes in, you look into fixing it. If at some point (either before or after you fix it), you notice that it should be applied to a point release, you simply change the milestone to be the point release. The bot then takes care of reopening/closing as needed to make sure it goes into that point release. That seems pretty foolproof as a tracking mechanism.

It might be nice to automate cherry-picks more too, but that's a bit orthogonal. One critical point is that I think it is very important that there be a delay between the commit on master and the commit on the release branch, giving a person a chance (even a few days) to decide "yes this really is right for the point release". In between those two, the bot will make sure the issue stays open.

If we could set two milestones on an issue that would take care of needing to do a double-point release (Go 1.8.4 and Go 1.9.1). We probably can't, in which case maybe the bot could just know that if there's a release-branch.go1.9 and something is marked for Go 1.8.4, it also need to be fixed in the 1.9 release branch before being closed. Then as long as we issue point releases in pairs (as I expect we will start to do after Go 1.9) we still won't miss anything.

Russ

[Nigel Tao]

I haven't been following this discussion closely, but I just marked
https://github.com/golang/go/issues/19553 "image/png: panic in Decode"
with the Go1.8.1 Milestone.

That issue (introduced by a new image/png feature in 1.8) was fixed by
https://github.com/golang/go/commit/16663a85ba0b2814c47f54ddfdcb45782e10dc42
"image/png: decode Gray8 transparent images."

Do I have to do anything else to have that (automatically, manually??)
considered for cherry-picking for 1.8.1?

[Russ Cox]

As long as it has the milestone, we'll do something with it before the release.

Thanks.

Russ

[Russ Cox]

Brad filed golang.org/issue/19776 for this.

[Keith Randall]

Just ran into this for golang.org/issue/19940.  Do we have explicit instructions I can point CL authors to somewhere?  If not, should I add a section to https://golang.org/doc/contribute.html ?

[Russ Cox]

I don't think there are explicit instructions. They should be something like "If we should consider including the bug fix in the next point release, set the milestone to that point release. A bot will take care of reopening the issue and tracking it into that release."