Skip to content

Commit 458b48b

Browse files
tstrombergtstromberg
authored
Merge pull request #101 from tstromberg/main
Use a smaller blocking dot for bot-driven PRs
2 parents 9c722c7 + 6716359 commit 458b48b

File tree

6 files changed

+130
-23
lines changed

6 files changed

+130
-23
lines changed

‎cmd/review-goose/filtering_test.go‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package main
22

33
import (
4+
"strings"
45
"testing"
56
"time"
67
)
@@ -221,3 +222,90 @@ func TestIsAlreadyTrackedAsBlocked(t *testing.T) {
221222
})
222223
}
223224
}
225+
226+
// TestBotPRsSortedAfterHumans tests that human-authored PRs appear before bot-authored PRs
227+
func TestBotPRsSortedAfterHumans(t *testing.T) {
228+
now := time.Now()
229+
230+
app := &App{
231+
incoming: []PR{
232+
{Repository: "org/repo1", Number: 1, Author: "dependabot[bot]", AuthorBot: true, NeedsReview: true, UpdatedAt: now},
233+
{Repository: "org/repo2", Number: 2, Author: "human-dev", AuthorBot: false, NeedsReview: true, UpdatedAt: now.Add(-1 * time.Hour)},
234+
{Repository: "org/repo3", Number: 3, Author: "renovate[bot]", AuthorBot: true, NeedsReview: true, UpdatedAt: now.Add(-2 * time.Hour)},
235+
{Repository: "org/repo4", Number: 4, Author: "another-human", AuthorBot: false, NeedsReview: true, UpdatedAt: now.Add(-3 * time.Hour)},
236+
},
237+
hiddenOrgs: map[string]bool{},
238+
stateManager: NewPRStateManager(now),
239+
systrayInterface: &MockSystray{},
240+
}
241+
242+
titles := app.generatePRSectionTitles(app.incoming, "Incoming", map[string]bool{}, false)
243+
244+
if len(titles) != 4 {
245+
t.Fatalf("Expected 4 titles, got %d", len(titles))
246+
}
247+
248+
// Human PRs should come first (repo2 and repo4), then bot PRs (repo1 and repo3)
249+
// Within each group, sorted by UpdatedAt (most recent first)
250+
expectedOrder := []string{"repo2", "repo4", "repo1", "repo3"}
251+
for i, expected := range expectedOrder {
252+
if !strings.Contains(titles[i], expected) {
253+
t.Errorf("Title %d: expected to contain %q, got %q", i, expected, titles[i])
254+
}
255+
}
256+
}
257+
258+
// TestBotPRsGetSmallerIcon tests that bot PRs get a smaller dot icon instead of the block
259+
func TestBotPRsGetSmallerIcon(t *testing.T) {
260+
now := time.Now()
261+
262+
app := &App{
263+
incoming: []PR{
264+
{
265+
Repository: "org/repo1",
266+
Number: 1,
267+
Author: "dependabot[bot]",
268+
AuthorBot: true,
269+
NeedsReview: true,
270+
IsBlocked: true,
271+
UpdatedAt: now,
272+
},
273+
{
274+
Repository: "org/repo2",
275+
Number: 2,
276+
Author: "human-dev",
277+
AuthorBot: false,
278+
NeedsReview: true,
279+
IsBlocked: true,
280+
UpdatedAt: now,
281+
},
282+
},
283+
hiddenOrgs: map[string]bool{},
284+
stateManager: NewPRStateManager(now),
285+
systrayInterface: &MockSystray{},
286+
}
287+
288+
titles := app.generatePRSectionTitles(app.incoming, "Incoming", map[string]bool{}, false)
289+
290+
if len(titles) != 2 {
291+
t.Fatalf("Expected 2 titles, got %d", len(titles))
292+
}
293+
294+
// Human PR should come first with block icon
295+
humanTitle := titles[0]
296+
if !strings.Contains(humanTitle, "repo2") {
297+
t.Errorf("Expected human PR (repo2) first, got: %s", humanTitle)
298+
}
299+
if !strings.HasPrefix(humanTitle, "■") {
300+
t.Errorf("Expected human PR to have block icon (■), got: %s", humanTitle)
301+
}
302+
303+
// Bot PR should come second with smaller dot
304+
botTitle := titles[1]
305+
if !strings.Contains(botTitle, "repo1") {
306+
t.Errorf("Expected bot PR (repo1) second, got: %s", botTitle)
307+
}
308+
if !strings.HasPrefix(botTitle, "·") {
309+
t.Errorf("Expected bot PR to have smaller dot (·), got: %s", botTitle)
310+
}
311+
}

‎cmd/review-goose/github.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,7 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
591591
}
592592

593593
// Update the PR in the slices directly
594+
authorBot := result.turnData.PullRequest.AuthorBot
594595
if result.isOwner {
595596
for i := range *outgoing {
596597
if (*outgoing)[i].URL != result.url {
@@ -602,6 +603,7 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
602603
(*outgoing)[i].ActionKind = actionKind
603604
(*outgoing)[i].TestState = testState
604605
(*outgoing)[i].WorkflowState = workflowState
606+
(*outgoing)[i].AuthorBot = authorBot
605607
break
606608
}
607609
} else {
@@ -615,6 +617,7 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
615617
(*incoming)[i].ActionKind = actionKind
616618
(*incoming)[i].TestState = testState
617619
(*incoming)[i].WorkflowState = workflowState
620+
(*incoming)[i].AuthorBot = authorBot
618621
break
619622
}
620623
}

‎cmd/review-goose/main.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ type PR struct {
9090
IsDraft bool
9191
IsBlocked bool
9292
NeedsReview bool
93+
AuthorBot bool // True if the author is a bot (dependabot, renovate, etc.)
9394
}
9495

9596
// App holds the application state.

‎cmd/review-goose/ui.go‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ func (app *App) setTrayTitle() {
220220

221221
// addPRSection adds a section of PRs to the menu.
222222
//
223-
//nolint:maintidx // Function complexity is inherent to PR menu building logic
223+
//nolint:maintidx,gocognit // Function complexity is inherent to PR menu building logic
224224
func (app *App) addPRSection(ctx context.Context, prs []PR, sectionTitle string, blockedCount int) {
225225
slog.Debug("[MENU] addPRSection called",
226226
"section", sectionTitle,
@@ -237,7 +237,7 @@ func (app *App) addPRSection(ctx context.Context, prs []PR, sectionTitle string,
237237
header := app.systrayInterface.AddMenuItem(headerText, "")
238238
header.Disable()
239239

240-
// Sort PRs with blocked ones first - inline for simplicity
240+
// Sort PRs with blocked ones first, humans before bots - inline for simplicity
241241
sortedPRs := make([]PR, len(prs))
242242
copy(sortedPRs, prs)
243243
sort.SliceStable(sortedPRs, func(i, j int) bool {
@@ -248,7 +248,11 @@ func (app *App) addPRSection(ctx context.Context, prs []PR, sectionTitle string,
248248
if sortedPRs[i].IsBlocked != sortedPRs[j].IsBlocked {
249249
return sortedPRs[i].IsBlocked // true (blocked) comes before false
250250
}
251-
// Second priority: more recent PRs first
251+
// Second priority: human PRs before bot PRs
252+
if sortedPRs[i].AuthorBot != sortedPRs[j].AuthorBot {
253+
return !sortedPRs[i].AuthorBot // false (human) comes before true (bot)
254+
}
255+
// Third priority: more recent PRs first
252256
return sortedPRs[i].UpdatedAt.After(sortedPRs[j].UpdatedAt)
253257
})
254258

@@ -342,8 +346,12 @@ func (app *App) addPRSection(ctx context.Context, prs []PR, sectionTitle string,
342346
"remaining", blockedPRIconDuration-timeSinceBlocked)
343347
}
344348
} else {
345-
// Use block/square icon for blocked PRs
346-
title = fmt.Sprintf("■ %s", title)
349+
// Use smaller dot for bot PRs, block icon for humans
350+
if sortedPRs[prIndex].AuthorBot {
351+
title = fmt.Sprintf("· %s", title)
352+
} else {
353+
title = fmt.Sprintf("■ %s", title)
354+
}
347355
// Log when we transition from emoji to block icon
348356
if hasState && !prState.FirstBlockedAt.IsZero() {
349357
timeSinceBlocked := time.Since(prState.FirstBlockedAt)
@@ -478,10 +486,13 @@ func (app *App) generateMenuTitles() []string {
478486
func (app *App) generatePRSectionTitles(prs []PR, sectionTitle string, hiddenOrgs map[string]bool, hideStale bool) []string {
479487
var titles []string
480488

481-
// Sort PRs by UpdatedAt (most recent first)
489+
// Sort PRs: humans before bots, then by UpdatedAt (most recent first)
482490
sortedPRs := make([]PR, len(prs))
483491
copy(sortedPRs, prs)
484492
sort.Slice(sortedPRs, func(i, j int) bool {
493+
if sortedPRs[i].AuthorBot != sortedPRs[j].AuthorBot {
494+
return !sortedPRs[i].AuthorBot // false (human) comes before true (bot)
495+
}
485496
return sortedPRs[i].UpdatedAt.After(sortedPRs[j].UpdatedAt)
486497
})
487498

@@ -546,8 +557,12 @@ func (app *App) generatePRSectionTitles(prs []PR, sectionTitle string, hiddenOrg
546557
"remaining", blockedPRIconDuration-timeSinceBlocked)
547558
}
548559
} else {
549-
// Use block/square icon for blocked PRs
550-
title = fmt.Sprintf("■ %s", title)
560+
// Use smaller dot for bot PRs, block icon for humans
561+
if sortedPRs[prIndex].AuthorBot {
562+
title = fmt.Sprintf("· %s", title)
563+
} else {
564+
title = fmt.Sprintf("■ %s", title)
565+
}
551566
// Log when we use block icon instead of emoji
552567
if hasState && !prState.FirstBlockedAt.IsZero() {
553568
timeSinceBlocked := time.Since(prState.FirstBlockedAt)

‎go.mod‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ require (
1010
github.com/gen2brain/beeep v0.11.1
1111
github.com/godbus/dbus/v5 v5.2.0
1212
github.com/google/go-github/v57 v57.0.0
13-
golang.org/x/image v0.33.0
14-
golang.org/x/oauth2 v0.33.0
13+
golang.org/x/image v0.34.0
14+
golang.org/x/oauth2 v0.34.0
1515
)
1616

1717
require (
@@ -26,7 +26,7 @@ require (
2626
github.com/sergeymakinen/go-ico v1.0.0 // indirect
2727
github.com/tadvi/systray v0.0.0-20190226123456-11a2b8fa57af // indirect
2828
github.com/tevino/abool v0.0.0-20220530134649-2bfc934cb23c // indirect
29-
golang.org/x/net v0.47.0 // indirect
30-
golang.org/x/sys v0.38.0 // indirect
31-
golang.org/x/text v0.31.0 // indirect
29+
golang.org/x/net v0.48.0 // indirect
30+
golang.org/x/sys v0.39.0 // indirect
31+
golang.org/x/text v0.32.0 // indirect
3232
)

‎go.sum‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,18 +51,18 @@ github.com/tadvi/systray v0.0.0-20190226123456-11a2b8fa57af h1:6yITBqGTE2lEeTPG0
5151
github.com/tadvi/systray v0.0.0-20190226123456-11a2b8fa57af/go.mod h1:4F09kP5F+am0jAwlQLddpoMDM+iewkxxt6nxUQ5nq5o=
5252
github.com/tevino/abool v0.0.0-20220530134649-2bfc934cb23c h1:coVla7zpsycc+kA9NXpcvv2E4I7+ii6L5hZO2S6C3kw=
5353
github.com/tevino/abool v0.0.0-20220530134649-2bfc934cb23c/go.mod h1:qc66Pna1RiIsPa7O4Egxxs9OqkuxDX55zznh9K07Tzg=
54-
golang.org/x/image v0.33.0 h1:LXRZRnv1+zGd5XBUVRFmYEphyyKJjQjCRiOuAP3sZfQ=
55-
golang.org/x/image v0.33.0/go.mod h1:DD3OsTYT9chzuzTQt+zMcOlBHgfoKQb1gry8p76Y1sc=
56-
golang.org/x/net v0.47.0 h1:Mx+4dIFzqraBXUugkia1OOvlD6LemFo1ALMHjrXDOhY=
57-
golang.org/x/net v0.47.0/go.mod h1:/jNxtkgq5yWUGYkaZGqo27cfGZ1c5Nen03aYrrKpVRU=
58-
golang.org/x/oauth2 v0.33.0 h1:4Q+qn+E5z8gPRJfmRy7C2gGG3T4jIprK6aSYgTXGRpo=
59-
golang.org/x/oauth2 v0.33.0/go.mod h1:lzm5WQJQwKZ3nwavOZ3IS5Aulzxi68dUSgRHujetwEA=
54+
golang.org/x/image v0.34.0 h1:33gCkyw9hmwbZJeZkct8XyR11yH889EQt/QH4VmXMn8=
55+
golang.org/x/image v0.34.0/go.mod h1:2RNFBZRB+vnwwFil8GkMdRvrJOFd1AzdZI6vOY+eJVU=
56+
golang.org/x/net v0.48.0 h1:zyQRTTrjc33Lhh0fBgT/H3oZq9WuvRR5gPC70xpDiQU=
57+
golang.org/x/net v0.48.0/go.mod h1:+ndRgGjkh8FGtu1w1FGbEC31if4VrNVMuKTgcAAnQRY=
58+
golang.org/x/oauth2 v0.34.0 h1:hqK/t4AKgbqWkdkcAeI8XLmbK+4m4G5YeQRrmiotGlw=
59+
golang.org/x/oauth2 v0.34.0/go.mod h1:lzm5WQJQwKZ3nwavOZ3IS5Aulzxi68dUSgRHujetwEA=
6060
golang.org/x/sys v0.0.0-20200515095857-1151b9dac4a9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
6161
golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
62-
golang.org/x/sys v0.38.0 h1:3yZWxaJjBmCWXqhN1qh02AkOnCQ1poK6oF+a7xWL6Gc=
63-
golang.org/x/sys v0.38.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
64-
golang.org/x/text v0.31.0 h1:aC8ghyu4JhP8VojJ2lEHBnochRno1sgL6nEi9WGFGMM=
65-
golang.org/x/text v0.31.0/go.mod h1:tKRAlv61yKIjGGHX/4tP1LTbc13YSec1pxVEWXzfoeM=
62+
golang.org/x/sys v0.39.0 h1:CvCKL8MeisomCi6qNZ+wbb0DN9E5AATixKsvNtMoMFk=
63+
golang.org/x/sys v0.39.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
64+
golang.org/x/text v0.32.0 h1:ZD01bjUt1FQ9WJ0ClOL5vxgxOI/sVCNgX1YtKwcY0mU=
65+
golang.org/x/text v0.32.0/go.mod h1:o/rUWzghvpD5TXrTIBuJU77MTaN0ljMWE47kxGJQ7jY=
6666
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
6767
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
6868
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

0 commit comments

Comments
 (0)