From 0f9d7e7c36246e3530356c21f13d8efe79dad3d3 Mon Sep 17 00:00:00 2001
From: Abhinav Gupta <mail@abhinavg.net>
Date: Sat, 14 Sep 2024 07:21:10 -0700
Subject: [PATCH] repo sync: Use ChangesAreMerged to batch check submitted
 branches (#400)

Make use of the ChangesAreMerged to make a single network request
for all submitted branches, rather than one per branch.

We still need to make one request per tracked branch for now.
---
 .../unreleased/Changed-20240913-185002.yaml   |  3 +
 repo_sync.go                                  | 97 +++++++++----------
 2 files changed, 51 insertions(+), 49 deletions(-)
 create mode 100644 .changes/unreleased/Changed-20240913-185002.yaml

diff --git a/.changes/unreleased/Changed-20240913-185002.yaml b/.changes/unreleased/Changed-20240913-185002.yaml
new file mode 100644
index 00000000..c1d869b6
--- /dev/null
+++ b/.changes/unreleased/Changed-20240913-185002.yaml
@@ -0,0 +1,3 @@
+kind: Changed
+body: 'repo sync: Reduce the number of network requests made to check status of submitted branches.'
+time: 2024-09-13T18:50:02.036293-07:00
diff --git a/repo_sync.go b/repo_sync.go
index 25a21cf4..8746d931 100644
--- a/repo_sync.go
+++ b/repo_sync.go
@@ -243,49 +243,64 @@ func (cmd *repoSyncCmd) deleteMergedBranches(
 	// 2. Branches that the user submitted PRs for manually
 	//    with 'gh pr create' or similar.
 	//
-	// For the first, we can perform a cheaper API call to check the CR status.
+	// For the first, we can perform a cheap API call to check the CR status.
 	// For the second, we need to find recently merged PRs with that branch
 	// name, and match the remote head SHA to the branch head SHA.
 	//
 	// We'll try to do these checks concurrently.
 
-	submittedch := make(chan *submittedBranch)
-	trachedch := make(chan *trackedBranch)
+	var (
+		submittedBranches []*submittedBranch
+		trackedBranches   []*trackedBranch
+	)
+	for _, b := range knownBranches {
+		if b.Change != nil {
+			b := &submittedBranch{
+				Name:   b.Name,
+				Change: b.Change.ChangeID(),
+			}
+			submittedBranches = append(submittedBranches, b)
+		} else {
+			// TODO:
+			// Filter down to only branches that have
+			// a remote tracking branch:
+			// either $remote/$UpstreamBranch or $remote/$branch exists.
+			b := &trackedBranch{Name: b.Name}
+			trackedBranches = append(trackedBranches, b)
+		}
+	}
 
 	var wg sync.WaitGroup
-	for range min(runtime.GOMAXPROCS(0), len(knownBranches)) {
+	if len(submittedBranches) > 0 {
 		wg.Add(1)
 		go func() {
 			defer wg.Done()
 
-			// We'll nil these out once they're closed.
-			submittedch := submittedch
-			trachedch := trachedch
-
-			for submittedch != nil || trachedch != nil {
-				select {
-				case b, ok := <-submittedch:
-					if !ok {
-						submittedch = nil
-						continue
-					}
+			changeIDs := make([]forge.ChangeID, len(submittedBranches))
+			for i, b := range submittedBranches {
+				changeIDs[i] = b.Change
+			}
 
-					// TODO: Once we're recording GraphQL IDs in the store,
-					// we can combine all submitted PRs into one query.
-					merged, err := remoteRepo.ChangesAreMerged(ctx, []forge.ChangeID{b.Change})
-					if err != nil {
-						log.Error("Failed to query CR status", "change", b.Change, "error", err)
-						continue
-					}
+			merged, err := remoteRepo.ChangesAreMerged(ctx, changeIDs)
+			if err != nil {
+				log.Error("Failed to query CR status", "error", err)
+				return
+			}
 
-					b.Merged = merged[0]
+			for i, m := range merged {
+				submittedBranches[i].Merged = m
+			}
+		}()
+	}
 
-				case b, ok := <-trachedch:
-					if !ok {
-						trachedch = nil
-						continue
-					}
+	if len(trackedBranches) > 0 {
+		trackedch := make(chan *trackedBranch)
+		for range min(runtime.GOMAXPROCS(0), len(trackedBranches)) {
+			wg.Add(1)
+			go func() {
+				defer wg.Done()
 
+				for b := range trackedch {
 					changes, err := remoteRepo.FindChangesByBranch(ctx, b.Name, forge.FindChangesOptions{
 						Limit: 3,
 						State: forge.ChangeMerged,
@@ -313,30 +328,14 @@ func (cmd *repoSyncCmd) deleteMergedBranches(
 					}
 
 				}
-			}
-		}()
-	}
+			}()
+		}
 
-	var (
-		submittedBranches []*submittedBranch
-		trackedBranches   []*trackedBranch
-	)
-	for _, b := range knownBranches {
-		if b.Change != nil {
-			b := &submittedBranch{
-				Name:   b.Name,
-				Change: b.Change.ChangeID(),
-			}
-			submittedBranches = append(submittedBranches, b)
-			submittedch <- b
-		} else {
-			b := &trackedBranch{Name: b.Name}
-			trackedBranches = append(trackedBranches, b)
-			trachedch <- b
+		for _, b := range trackedBranches {
+			trackedch <- b
 		}
+		close(trackedch)
 	}
-	close(submittedch)
-	close(trachedch)
 	wg.Wait()
 
 	var branchesToDelete []string