cmd/gopherbot: handle comment threads with 3+ depth in Auto-Submit task
The current logic to find if a CL has any unresolved threads is to
iterate over all comments in chronological order, and use the resolved
state of most recent comments to update the overall resolved state of
the corresponding thread. Threads are identified by the ID of the root
(earliest) comment whose InReplyTo field is zero.
When determining the thread ID that a reply is associated with, it's
important to follow the loop of InReplyTo pointers all way to the top
of the thread. That is, if there's a thread with 3 comments in total:
A ← B ← C
(Where A is an initial comment, B is a reply to it, and C is a reply
to B.)
Then the resolved state of comment C needs to update the resolved state
of thread A, even though comment C is a direct reply to comment B.
The existing code worked for threads with depths 1 and 2, but not 3+.
To handle threads with deeper nesting, add a 'roots' map that tracks
the root ID for each comment seen during the chronological iteration.
I thought this would apply to the equivalent logic in coordinator, and
probably should have checked sooner, as then I'd find this was already
implemented there in CL 317971. This change applies some comments and
code style decisions from coordinator's listPatchSetThreads function
so they're more in sync.
For golang/go#48021.
Change-Id: I93baff45aae511a60ccfce4e97b05d2a1b358e1f
Reviewed-on: https://go-review.googlesource.com/c/build/+/386294
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/cmd/gopherbot/gopherbot.go b/cmd/gopherbot/gopherbot.go
index 27b8b54..3ac8d7f 100644
--- a/cmd/gopherbot/gopherbot.go
+++ b/cmd/gopherbot/gopherbot.go
@@ -2308,16 +2308,29 @@
return err
}
for _, commentSet := range comments {
+ // The API doesn't sort comments chronologically, but "the state of
+ // resolution of a comment thread is stored in the last comment in that
+ // thread chronologically", so first of all sort them by time.
sort.Slice(commentSet, func(i, j int) bool {
return commentSet[i].Updated.Time().Before(commentSet[j].Updated.Time())
})
+
+ // roots is a map of message IDs to their thread root.
+ roots := make(map[string]string)
threads := make(map[string]bool)
for _, c := range commentSet {
- id := c.ID
- if c.InReplyTo != "" {
- id = c.InReplyTo
+ if c.InReplyTo == "" {
+ roots[c.ID] = c.ID
+ threads[c.ID] = *c.Unresolved
+ continue
}
- threads[id] = *c.Unresolved
+
+ root, ok := roots[c.InReplyTo]
+ if !ok {
+ return fmt.Errorf("failed to determine unresolved state in CL %d: comment %q (parent of comment %q) is not found", cl.Number, c.InReplyTo, c.ID)
+ }
+ roots[c.ID] = root
+ threads[root] = *c.Unresolved
}
for _, unresolved := range threads {
if unresolved {
