Skip to content

Commit 5f3c067

Browse files
committed
prevent unnecessary menu builds
1 parent adb9eed commit 5f3c067

File tree

7 files changed

+230
-224
lines changed

7 files changed

+230
-224
lines changed

github.go

Lines changed: 52 additions & 140 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,16 @@ func (*App) githubToken(ctx context.Context) (string, error) {
122122
// fetchPRs retrieves all PRs involving the current user.
123123
// It returns GitHub data immediately and starts Turn API queries in the background.
124124
func (app *App) fetchPRs(ctx context.Context) (incoming []PR, outgoing []PR, err error) {
125+
return app.fetchPRsInternal(ctx, false)
126+
}
127+
128+
// fetchPRsWithWait fetches PRs and waits for Turn data to complete.
129+
func (app *App) fetchPRsWithWait(ctx context.Context) (incoming []PR, outgoing []PR, err error) {
130+
return app.fetchPRsInternal(ctx, true)
131+
}
132+
133+
// fetchPRsInternal is the common implementation for PR fetching.
134+
func (app *App) fetchPRsInternal(ctx context.Context, waitForTurn bool) (incoming []PR, outgoing []PR, err error) {
125135
// Use targetUser if specified, otherwise use authenticated user
126136
user := app.currentUser.GetLogin()
127137
if app.targetUser != "" {
@@ -236,8 +246,19 @@ func (app *App) fetchPRs(ctx context.Context) (incoming []PR, outgoing []PR, err
236246
log.Printf("[GITHUB] Outgoing PR: %s", pr.URL)
237247
}
238248

239-
// Start Turn API queries in background
240-
go app.fetchTurnDataAsync(ctx, result.Issues, user)
249+
// Fetch Turn API data
250+
if waitForTurn {
251+
// Synchronous - wait for Turn data
252+
log.Println("[TURN] Fetching Turn API data synchronously before building menu...")
253+
app.fetchTurnDataSync(ctx, result.Issues, user, &incoming, &outgoing)
254+
} else {
255+
// Asynchronous - start in background
256+
app.mu.Lock()
257+
app.loadingTurnData = true
258+
app.pendingTurnResults = make([]TurnResult, 0) // Reset buffer
259+
app.mu.Unlock()
260+
go app.fetchTurnDataAsync(ctx, result.Issues, user)
261+
}
241262

242263
return incoming, outgoing, nil
243264
}
@@ -270,122 +291,6 @@ func (app *App) updatePRData(url string, needsReview bool, isOwner bool, actionR
270291
return nil
271292
}
272293

273-
// fetchPRsWithWait fetches PRs and waits for Turn data to complete.
274-
func (app *App) fetchPRsWithWait(ctx context.Context) (incoming []PR, outgoing []PR, err error) {
275-
// Use targetUser if specified, otherwise use authenticated user
276-
user := app.currentUser.GetLogin()
277-
if app.targetUser != "" {
278-
user = app.targetUser
279-
}
280-
281-
// Single query to get all PRs involving the user
282-
query := fmt.Sprintf("is:open is:pr involves:%s archived:false", user)
283-
284-
const perPage = 100
285-
opts := &github.SearchOptions{
286-
ListOptions: github.ListOptions{PerPage: perPage},
287-
Sort: "updated",
288-
Order: "desc",
289-
}
290-
291-
log.Printf("Searching for PRs with query: %s", query)
292-
searchStart := time.Now()
293-
294-
var result *github.IssuesSearchResult
295-
var resp *github.Response
296-
err = retry.Do(func() error {
297-
// Create timeout context for GitHub API call
298-
githubCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
299-
defer cancel()
300-
301-
var retryErr error
302-
result, resp, retryErr = app.client.Search.Issues(githubCtx, query, opts)
303-
if retryErr != nil {
304-
// Enhanced error handling with specific cases
305-
if resp != nil {
306-
const (
307-
httpStatusUnauthorized = 401
308-
httpStatusForbidden = 403
309-
httpStatusUnprocessable = 422
310-
)
311-
switch resp.StatusCode {
312-
case httpStatusForbidden:
313-
if resp.Header.Get("X-Ratelimit-Remaining") == "0" {
314-
resetTime := resp.Header.Get("X-Ratelimit-Reset")
315-
log.Printf("GitHub API rate limited, reset at: %s (will retry)", resetTime)
316-
return retryErr // Retry on rate limit
317-
}
318-
log.Print("GitHub API access forbidden (check token permissions)")
319-
return retry.Unrecoverable(fmt.Errorf("github API access forbidden: %w", retryErr))
320-
case httpStatusUnauthorized:
321-
log.Print("GitHub API authentication failed (check token)")
322-
return retry.Unrecoverable(fmt.Errorf("github API authentication failed: %w", retryErr))
323-
case httpStatusUnprocessable:
324-
log.Printf("GitHub API query invalid: %s", query)
325-
return retry.Unrecoverable(fmt.Errorf("github API query invalid: %w", retryErr))
326-
default:
327-
log.Printf("GitHub API error (status %d): %v (will retry)", resp.StatusCode, retryErr)
328-
}
329-
} else {
330-
// Likely network error - retry these
331-
log.Printf("GitHub API network error: %v (will retry)", retryErr)
332-
}
333-
return retryErr
334-
}
335-
return nil
336-
},
337-
retry.Attempts(maxRetries),
338-
retry.DelayType(retry.BackOffDelay),
339-
retry.MaxDelay(maxRetryDelay),
340-
retry.OnRetry(func(n uint, err error) {
341-
log.Printf("GitHub Search.Issues retry %d/%d: %v", n+1, maxRetries, err)
342-
}),
343-
retry.Context(ctx),
344-
)
345-
if err != nil {
346-
return nil, nil, fmt.Errorf("search PRs after %d retries: %w", maxRetries, err)
347-
}
348-
349-
log.Printf("GitHub search completed in %v, found %d PRs", time.Since(searchStart), len(result.Issues))
350-
351-
// Limit PRs for performance
352-
if len(result.Issues) > maxPRsToProcess {
353-
log.Printf("Limiting to %d PRs for performance (total: %d)", maxPRsToProcess, len(result.Issues))
354-
result.Issues = result.Issues[:maxPRsToProcess]
355-
}
356-
357-
// Process GitHub results
358-
for _, issue := range result.Issues {
359-
if !issue.IsPullRequest() {
360-
continue
361-
}
362-
repo := strings.TrimPrefix(issue.GetRepositoryURL(), "https://api.github.com/repos/")
363-
364-
pr := PR{
365-
Title: issue.GetTitle(),
366-
URL: issue.GetHTMLURL(),
367-
Repository: repo,
368-
Number: issue.GetNumber(),
369-
UpdatedAt: issue.GetUpdatedAt().Time,
370-
}
371-
372-
// Categorize as incoming or outgoing
373-
if issue.GetUser().GetLogin() == user {
374-
outgoing = append(outgoing, pr)
375-
} else {
376-
incoming = append(incoming, pr)
377-
}
378-
}
379-
380-
log.Printf("[GITHUB] Found %d incoming, %d outgoing PRs from GitHub", len(incoming), len(outgoing))
381-
382-
// Now fetch Turn data synchronously
383-
log.Println("[TURN] Fetching Turn API data synchronously before building menu...")
384-
app.fetchTurnDataSync(ctx, result.Issues, user, &incoming, &outgoing)
385-
386-
return incoming, outgoing, nil
387-
}
388-
389294
// fetchTurnDataSync fetches Turn API data synchronously and updates PRs directly.
390295
func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, user string, incoming *[]PR, outgoing *[]PR) {
391296
turnStart := time.Now()
@@ -533,11 +438,7 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue,
533438
turnFailures := 0
534439
updatesApplied := 0
535440

536-
// Batch updates to reduce menu rebuilds
537-
updateBatch := 0
538-
const batchSize = 10
539-
lastUpdateTime := time.Now()
540-
const minUpdateInterval = 500 * time.Millisecond
441+
// Process results as they arrive and buffer them
541442

542443
for result := range results {
543444
if result.err == nil && result.turnData != nil && result.turnData.PRState.UnblockAction != nil {
@@ -554,22 +455,20 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue,
554455
log.Printf("[TURN] No UnblockAction found for user %s on %s", user, result.url)
555456
}
556457

557-
// Update the PR in our lists
558-
pr := app.updatePRData(result.url, needsReview, result.isOwner, actionReason)
458+
// Buffer the Turn result instead of applying immediately
459+
turnResult := TurnResult{
460+
URL: result.url,
461+
NeedsReview: needsReview,
462+
IsOwner: result.isOwner,
463+
ActionReason: actionReason,
464+
}
559465

560-
if pr != nil {
561-
updatesApplied++
562-
updateBatch++
563-
log.Printf("[TURN] Turn data received for %s (needsReview=%v, actionReason=%q)", result.url, needsReview, actionReason)
466+
app.mu.Lock()
467+
app.pendingTurnResults = append(app.pendingTurnResults, turnResult)
468+
app.mu.Unlock()
564469

565-
// Periodically update tray title
566-
if updateBatch >= batchSize || time.Since(lastUpdateTime) >= minUpdateInterval {
567-
log.Printf("[TURN] Batch update threshold reached (%d updates), updating title", updateBatch)
568-
app.setTrayTitle()
569-
updateBatch = 0
570-
lastUpdateTime = time.Now()
571-
}
572-
}
470+
updatesApplied++
471+
log.Printf("[TURN] Turn data received for %s (needsReview=%v, actionReason=%q) - buffered", result.url, needsReview, actionReason)
573472
} else if result.err != nil {
574473
turnFailures++
575474
}
@@ -578,9 +477,22 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue,
578477
log.Printf("[TURN] Turn API queries completed in %v (%d/%d succeeded, %d PRs updated)",
579478
time.Since(turnStart), turnSuccesses, turnSuccesses+turnFailures, updatesApplied)
580479

581-
// Rebuild menu with final Turn data if menu is already initialized
480+
// Apply all buffered Turn results at once
481+
app.mu.Lock()
482+
pendingResults := app.pendingTurnResults
483+
app.pendingTurnResults = nil
484+
app.loadingTurnData = false
485+
app.mu.Unlock()
486+
487+
log.Printf("[TURN] Applying %d buffered Turn results", len(pendingResults))
488+
for _, result := range pendingResults {
489+
app.updatePRData(result.URL, result.NeedsReview, result.IsOwner, result.ActionReason)
490+
}
491+
492+
// Update tray title and menu with final Turn data if menu is already initialized
493+
app.setTrayTitle()
582494
if app.menuInitialized {
583-
log.Print("[TURN] Turn data loaded, rebuilding menu")
584-
app.rebuildMenu(ctx)
495+
log.Print("[TURN] Turn data loaded, checking if menu needs update")
496+
app.updateMenuIfChanged(ctx)
585497
}
586498
}

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ require (
1616
github.com/esiqveland/notify v0.13.3 // indirect
1717
github.com/go-ole/go-ole v1.3.0 // indirect
1818
github.com/godbus/dbus/v5 v5.1.0 // indirect
19+
github.com/google/go-cmp v0.7.0 // indirect
1920
github.com/google/go-querystring v1.1.0 // indirect
2021
github.com/jackmordaunt/icns/v3 v3.0.1 // indirect
2122
github.com/nfnt/resize v0.0.0-20180221191011-83c6a9932646 // indirect

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ github.com/godbus/dbus/v5 v5.1.0/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5x
1919
github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
2020
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
2121
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
22+
github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8=
23+
github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU=
2224
github.com/google/go-github/v57 v57.0.0 h1:L+Y3UPTY8ALM8x+TV0lg+IEBI+upibemtBD8Q9u7zHs=
2325
github.com/google/go-github/v57 v57.0.0/go.mod h1:s0omdnye0hvK/ecLvpsGfJMiRt85PimQh4oygmLIxHw=
2426
github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8=

0 commit comments

Comments
 (0)