Skip to content

Commit 96df98a

Browse files
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 <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]>
1 parent 92f7ca4 commit 96df98a

File tree

2 files changed

+16
-18
lines changed

2 files changed

+16
-18
lines changed

cmd/gopherbot/gopherbot.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2465,14 +2465,11 @@ func (b *gopherbot) autoSubmitCLs(ctx context.Context) error {
24652465
// If this change is part of a stack, we'd like to merge the stack
24662466
// in the correct order (i.e. from the bottom of the stack to the
24672467
// top), so we'll only merge the current change if every change
2468-
// below it in the stack is either merged, or abandoned. If a parent
2469-
// change is no longer current (the revision of the change that the
2470-
// child change is based on is no longer the current revision of
2471-
// that change) we won't merge the child. GetRelatedChanges gives us
2472-
// the stack from top to bottom (the order of the git commits, from
2473-
// newest to oldest, see Gerrit documentation for
2474-
// RelatedChangesInfo), so first we find our change in the stack,
2475-
// then check everything below it.
2468+
// below it in the stack is either merged, or abandoned.
2469+
// GetRelatedChanges gives us the stack from top to bottom (the
2470+
// order of the git commits, from newest to oldest, see Gerrit
2471+
// documentation for RelatedChangesInfo), so first we find our
2472+
// change in the stack, then check everything below it.
24762473
relatedChanges, err := b.gerrit.GetRelatedChanges(ctx, fmt.Sprint(cl.Number), "current")
24772474
if err != nil {
24782475
return err
@@ -2491,9 +2488,12 @@ func (b *gopherbot) autoSubmitCLs(ctx context.Context) error {
24912488
ci.Status != gerrit.ChangeStatusMerged {
24922489
return nil
24932490
}
2494-
if ci.CurrentRevisionNumber != ci.RevisionNumber {
2495-
return nil
2496-
}
2491+
// We do not check the revision number of merged/abandoned
2492+
// parents since, even if they are not current according to
2493+
// gerrit, if there were any merge conflicts, caused by the
2494+
// diffs between the revision this change was based on and
2495+
// the current revision, the change would not be considered
2496+
// submittable anyway.
24972497
}
24982498
}
24992499

gerrit/gerrit.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1051,13 +1051,11 @@ func (c *Client) GetRevisionActions(ctx context.Context, changeID, revision stri
10511051
//
10521052
// See https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#related-change-and-commit-info.
10531053
type RelatedChangeAndCommitInfo struct {
1054-
Project string `json:"project"`
1055-
ChangeID string `json:"change_id"`
1056-
ChangeNumber int32 `json:"_change_number"`
1057-
Commit CommitInfo `json:"commit"`
1058-
Status string `json:"status"`
1059-
RevisionNumber int32 `json:"_revision_number"`
1060-
CurrentRevisionNumber int32 `json:"_current_revision_number"`
1054+
Project string `json:"project"`
1055+
ChangeID string `json:"change_id"`
1056+
ChangeNumber int32 `json:"_change_number"`
1057+
Commit CommitInfo `json:"commit"`
1058+
Status string `json:"status"`
10611059
}
10621060

10631061
// RelatedChangesInfo contains information about a set of related changes.

0 commit comments

Comments
 (0)