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

gerrit: don't wipe Trybot-Result when updating commit messages #30893

Open
mvdan opened this issue Mar 17, 2019 · 5 comments

Comments

@mvdan
Copy link
Member

commented Mar 17, 2019

For example, take https://go-review.googlesource.com/c/go/+/167978. I forgot to add a line to the commit message, so I updated it in patchset 2. That wouldn't affect the trybots at all, so I left them running for patchset 1 only.

However, that has the unfortunate side effect that I've now lost the Trybot-Result label on my CL. I could re-run the trybots, but that seems a bit wasteful.

It would be nice if the label wasn't wiped when just the commit message is updated. That is, assuming that the trybots are never going to care about the commit message. I don't think they do, right now, nor do I see a reason why they should.

If this is something Gerrit just isn't designed to do, perhaps we could just have Gobot post the Trybot-Result label again if a patchset only updated the commit message.

/cc @andybons @bradfitz @dmitshur

@dmitshur

This comment has been minimized.

Copy link
Member

commented Mar 18, 2019

In what sense is it lost? I see a "TryBots are happy." message with a "TryBot-Result +1" vote at https://go-review.googlesource.com/c/go/+/167978#message-c36076efcbed3a38e87c5f6d4de374821854b7f5.

Are you referring to the top-level view of the CL not showing any votes?

@dmitshur dmitshur changed the title Gerrit: don't wipe Trybot-Result when updating commit messages gerrit: don't wipe Trybot-Result when updating commit messages Mar 18, 2019

@bradfitz

This comment has been minimized.

Copy link
Member

commented Mar 18, 2019

@dmitshur, yes, he's referring to the overall score on the CL. This is due to copyMinScore ... we have our Code-Review=+2 copied, but not +1 votes. See https://gerrit-review.googlesource.com/Documentation/config-labels.html#label_copyMinScore

But yes, the coordinator should & could check that a commit differs only in commit message and manually re-copy the vote without re-trying it. (But we shouldn't do the same with rebases, of course.)

@cuonglm

This comment has been minimized.

Copy link
Contributor

commented Mar 25, 2019

@mvdan @bradfitz How about the case when the commit only update comment lines?

It's great if we can retain the result in this case, too.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Mar 25, 2019

@Gnouc, that's not 100% guaranteed to result in the same results. (gofmt checks, allhelp.go staleness checks, etc)

@dmitshur

This comment has been minimized.

Copy link
Member

commented May 1, 2019

@heschik ran into this today in https://go-review.googlesource.com/c/go/+/174740/3#message-2ed0cb78dcc91bc5f91e7c424595f08bc212e465.

To avoid running into this again until this issue is resolved, don't update commit message right after starting a trybot run.

Edit: Perhaps it was different in that it wasn't just the label that was lost, but the trybot result message as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.