New issue
Advanced search Search tips

Issue 680613 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: ----



Sign in to add a comment

PG buildbucket plugin should display tryjobs after CL is merged

Project Member Reported by tandrii@chromium.org, Jan 12 2017

Issue description

I propose for plugin to show jobs from second to last* patchset if CL has been merged.

Since we use submit strategies like Cherry-Pick or Rebase Always, which insert a new patchset when merging (landing) a CL, the last patchset will never have associated buildbucket builds.
 

* which may not necessarily be N-1 because Gerrit internals may "skip" a patchset number in some occasions.
 
Cc: aga...@chromium.org rmis...@chromium.org
From my PoV, this is a regression vs current Rietveld. WDYT?

Comment 2 by aga...@chromium.org, Jan 12 2017

Labels: -Restrict-View-Google Milestone-Launch
Status: Available (was: Untriaged)
So this definitely works sometimes: https://chromium-review.googlesource.com/c/424623/
Here's where it doesn't work: https://chromium-review.googlesource.com/c/426840

They're both in the infra.git repo, using the same submit strategy (cherry pick). It seems like one was a "cleaner" cherrypick than the other, so gerrit recognized it as being trivial, while the other was classified as a "rework" (the default for normal patchset code changes made by humans) so the backwards crawling in the buildbucket plugin stopped.

Seems like the buildbucket plugin should grow some extra logic for "if the most recent patchset was the one where it got merged".

It's not clear how this will behave with the rebase-always strategy instead of cherry-pick. It might be better, or it might be the same. We should improve buildbucket anyway.

Comment 3 by aga...@chromium.org, Jan 18 2017

Owner: aga...@chromium.org
Status: Started (was: Available)
https://chromium-review.googlesource.com/430310
Labels: Pri-2
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/gerrit-plugins/buildbucket/+/76b8d7ecc77cbb7bbe01c714dc7956554111376d

commit 76b8d7ecc77cbb7bbe01c714dc7956554111376d
Author: Aaron Gable <agable@chromium.org>
Date: Wed Jan 18 22:12:29 2017

Skip autogenerated merge patchset during computation

Whenever the "Submit" button is clicked, gerrit generates a new
patchset. This patchset's commit message includes metadata (e.g.
who the reviewers were) in git footers. Depending on the submit
strategy and the state of the repo, this final patchset sometimes
counts as a trivial change, but sometimes it counts as a rework.
In the cases where it counts as a rework, the buildbucket plugin
would fail to traverse to older patchsets to find their tryjobs.

This change causes the plugin to always skip the most recent
patchset on merged/submitted changes. This is safe because Gerrit
will always generates that patchset (there are no merged CLs
without a finial patchset), and because that last patchset will
never have tryjobs of its own.

BUG= 680613 

Change-Id: I6bfc749301830bd57379268b215692ab245fc334
Reviewed-on: https://chromium-review.googlesource.com/430310
Reviewed-by: Andrew Bonventre <andybons@chromium.org>

[modify] https://crrev.com/76b8d7ecc77cbb7bbe01c714dc7956554111376d/src/main/resources/static/buildbucket.js

Status: Fixed (was: Started)

Sign in to add a comment