cmd/gopherbot: revise autosubmit behavior for stacks
When checking whether to autosubmit a change in a stack, ignore whether
the revision of merged/abandoned parents are current (base revision ==
current revision). There are multiple reasons a merged parent may not be
current (the most obvious being that the parent change was submitted,
which increases the revision number, but the child was not rebased onto
the new revision), but as long as the change is still considered
'submittable' by gerrit (i.e. there are no merge conflicts) this should
not materially affect our decision of whether or not to submit the
change (and matches what most users will do when manually submitting a
stack).
For golang/go#48021.
Change-Id: Iceff8a88ac3638671f36175d802254788d2470fd
Reviewed-on: https://go-review.googlesource.com/c/build/+/406237
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
diff --git a/cmd/gopherbot/gopherbot.go b/cmd/gopherbot/gopherbot.go
index 28c4c29..35e52ab 100644
--- a/cmd/gopherbot/gopherbot.go
+++ b/cmd/gopherbot/gopherbot.go
@@ -2465,14 +2465,11 @@
// If this change is part of a stack, we'd like to merge the stack
// in the correct order (i.e. from the bottom of the stack to the
// top), so we'll only merge the current change if every change
- // below it in the stack is either merged, or abandoned. If a parent
- // change is no longer current (the revision of the change that the
- // child change is based on is no longer the current revision of
- // that change) we won't merge the child. GetRelatedChanges gives us
- // the stack from top to bottom (the order of the git commits, from
- // newest to oldest, see Gerrit documentation for
- // RelatedChangesInfo), so first we find our change in the stack,
- // then check everything below it.
+ // below it in the stack is either merged, or abandoned.
+ // GetRelatedChanges gives us the stack from top to bottom (the
+ // order of the git commits, from newest to oldest, see Gerrit
+ // documentation for RelatedChangesInfo), so first we find our
+ // change in the stack, then check everything below it.
relatedChanges, err := b.gerrit.GetRelatedChanges(ctx, fmt.Sprint(cl.Number), "current")
if err != nil {
return err
@@ -2491,9 +2488,12 @@
ci.Status != gerrit.ChangeStatusMerged {
return nil
}
- if ci.CurrentRevisionNumber != ci.RevisionNumber {
- return nil
- }
+ // We do not check the revision number of merged/abandoned
+ // parents since, even if they are not current according to
+ // gerrit, if there were any merge conflicts, caused by the
+ // diffs between the revision this change was based on and
+ // the current revision, the change would not be considered
+ // submittable anyway.
}
}
diff --git a/gerrit/gerrit.go b/gerrit/gerrit.go
index 93de748..bdc93ab 100644
--- a/gerrit/gerrit.go
+++ b/gerrit/gerrit.go
@@ -1051,13 +1051,11 @@
//
// See https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#related-change-and-commit-info.
type RelatedChangeAndCommitInfo struct {
- Project string `json:"project"`
- ChangeID string `json:"change_id"`
- ChangeNumber int32 `json:"_change_number"`
- Commit CommitInfo `json:"commit"`
- Status string `json:"status"`
- RevisionNumber int32 `json:"_revision_number"`
- CurrentRevisionNumber int32 `json:"_current_revision_number"`
+ Project string `json:"project"`
+ ChangeID string `json:"change_id"`
+ ChangeNumber int32 `json:"_change_number"`
+ Commit CommitInfo `json:"commit"`
+ Status string `json:"status"`
}
// RelatedChangesInfo contains information about a set of related changes.
