Skip to content

Commit 3912b44

Browse files
authored
Merge pull request #13 from github/skarim/skip-queued-prs
Skip pushing branches with queued PRs
2 parents 0ba54a4 + dcb5885 commit 3912b44

File tree

14 files changed

+597
-64
lines changed

14 files changed

+597
-64
lines changed

cmd/push.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,20 @@ func runPush(cfg *config.Config, opts *pushOptions) error {
7171
}
7272
return ErrSilent
7373
}
74+
// Sync PR state to detect merged/queued PRs before pushing.
75+
syncStackPRs(cfg, s)
76+
7477
merged := s.MergedBranches()
7578
if len(merged) > 0 {
7679
cfg.Printf("Skipping %d merged %s", len(merged), plural(len(merged), "branch", "branches"))
7780
}
81+
queued := s.QueuedBranches()
82+
if len(queued) > 0 {
83+
cfg.Printf("Skipping %d queued %s", len(queued), plural(len(queued), "branch", "branches"))
84+
}
7885
activeBranches := activeBranchNames(s)
7986
if len(activeBranches) == 0 {
80-
cfg.Printf("No active branches to push (all merged)")
87+
cfg.Printf("No active branches to push (all merged or queued)")
8188
return nil
8289
}
8390
cfg.Printf("Pushing %d %s to %s...", len(activeBranches), plural(len(activeBranches), "branch", "branches"), remote)

cmd/submit.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,9 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error {
7777
return ErrAPIFailure
7878
}
7979

80+
// Sync PR state to detect merged/queued PRs before pushing.
81+
syncStackPRs(cfg, s)
82+
8083
// Push all active branches atomically
8184
remote, err := pickRemote(cfg, currentBranch, opts.remote)
8285
if err != nil {
@@ -89,9 +92,13 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error {
8992
if len(merged) > 0 {
9093
cfg.Printf("Skipping %d merged %s", len(merged), plural(len(merged), "branch", "branches"))
9194
}
95+
queued := s.QueuedBranches()
96+
if len(queued) > 0 {
97+
cfg.Printf("Skipping %d queued %s", len(queued), plural(len(queued), "branch", "branches"))
98+
}
9299
activeBranches := activeBranchNames(s)
93100
if len(activeBranches) == 0 {
94-
cfg.Printf("All branches are merged, nothing to submit")
101+
cfg.Printf("All branches are merged or queued, nothing to submit")
95102
return nil
96103
}
97104
cfg.Printf("Pushing %d %s to %s...", len(activeBranches), plural(len(activeBranches), "branch", "branches"), remote)
@@ -104,7 +111,7 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error {
104111
// correct base branch. This makes submit idempotent: running it again
105112
// fills gaps and fixes base branches before syncing the stack.
106113
for i, b := range s.Branches {
107-
if s.Branches[i].IsMerged() {
114+
if s.Branches[i].IsMerged() || s.Branches[i].IsQueued() {
108115
continue
109116
}
110117
baseBranch := s.ActiveBaseBranch(b.Branch)

cmd/sync.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,12 +152,20 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
152152
continue
153153
}
154154

155+
// Skip branches whose PRs are currently in a merge queue.
156+
if br.IsQueued() {
157+
ontoOldBase = originalRefs[br.Branch]
158+
needsOnto = true
159+
cfg.Successf("Skipping %s (PR %s queued)", br.Branch, cfg.PRLink(br.PullRequest.Number, br.PullRequest.URL))
160+
continue
161+
}
162+
155163
if needsOnto {
156-
// Find --onto target: first non-merged ancestor, or trunk.
164+
// Find --onto target: first non-merged/queued ancestor, or trunk.
157165
newBase := trunk
158166
for j := i - 1; j >= 0; j-- {
159167
b := s.Branches[j]
160-
if !b.IsMerged() {
168+
if !b.IsSkipped() {
161169
newBase = b.Branch
162170
break
163171
}
@@ -233,6 +241,9 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
233241
if mergedCount := len(s.MergedBranches()); mergedCount > 0 {
234242
cfg.Printf("Skipping %d merged %s", mergedCount, plural(mergedCount, "branch", "branches"))
235243
}
244+
if queuedCount := len(s.QueuedBranches()); queuedCount > 0 {
245+
cfg.Printf("Skipping %d queued %s", queuedCount, plural(queuedCount, "branch", "branches"))
246+
}
236247

237248
if len(branches) == 0 {
238249
cfg.Printf("No active branches to push (all merged)")
@@ -265,6 +276,10 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
265276
if b.IsMerged() {
266277
continue
267278
}
279+
if b.IsQueued() {
280+
cfg.Successf("PR %s (%s) — Queued", cfg.PRLink(b.PullRequest.Number, b.PullRequest.URL), b.Branch)
281+
continue
282+
}
268283
if b.PullRequest != nil {
269284
cfg.Successf("PR %s (%s) — Open", cfg.PRLink(b.PullRequest.Number, b.PullRequest.URL), b.Branch)
270285
} else {

cmd/utils.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ func resolveStack(sf *stack.StackFile, branch string, cfg *config.Config) (*stac
214214
// syncStackPRs discovers and updates pull request metadata for branches in a stack.
215215
// For each branch, it queries GitHub for the most recent PR and updates the
216216
// PullRequestRef including merge status. Branches with already-merged PRs are skipped.
217+
// The transient Queued flag is also populated from the API response.
217218
func syncStackPRs(cfg *config.Config, s *stack.Stack) {
218219
client, err := cfg.GitHubClient()
219220
if err != nil {
@@ -238,6 +239,7 @@ func syncStackPRs(cfg *config.Config, s *stack.Stack) {
238239
URL: pr.URL,
239240
Merged: pr.Merged,
240241
}
242+
b.Queued = pr.IsQueued()
241243
}
242244
}
243245

cmd/view.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,14 @@ func viewShort(cfg *config.Config, s *stack.Stack, currentBranch string) error {
7676
for i := len(s.Branches) - 1; i >= 0; i-- {
7777
b := s.Branches[i]
7878
merged := b.IsMerged()
79+
queued := b.IsQueued()
7980

80-
// Insert separator when transitioning from active to merged section
81+
// Insert separator when transitioning from active to queued section
82+
if queued && !merged && (i == len(s.Branches)-1 || (!s.Branches[i+1].IsQueued() && !s.Branches[i+1].IsMerged())) {
83+
cfg.Outf("├─── %s ────\n", cfg.ColorWarning("queued"))
84+
}
85+
86+
// Insert separator when transitioning from active/queued to merged section
8187
if merged && (i == len(s.Branches)-1 || !s.Branches[i+1].IsMerged()) {
8288
cfg.Outf("├─── %s ────\n", cfg.ColorMagenta("merged"))
8389
}
@@ -88,6 +94,8 @@ func viewShort(cfg *config.Config, s *stack.Stack, currentBranch string) error {
8894
cfg.Outf("» %s%s%s %s\n", cfg.ColorBold(b.Branch), indicator, prSuffix, cfg.ColorCyan("(current)"))
8995
} else if merged {
9096
cfg.Outf("│ %s%s%s\n", cfg.ColorGray(b.Branch), indicator, prSuffix)
97+
} else if queued {
98+
cfg.Outf("│ %s%s%s\n", cfg.ColorWarning(b.Branch), indicator, prSuffix)
9199
} else {
92100
cfg.Outf("├ %s%s%s\n", b.Branch, indicator, prSuffix)
93101
}
@@ -98,13 +106,18 @@ func viewShort(cfg *config.Config, s *stack.Stack, currentBranch string) error {
98106

99107
// branchStatusIndicator returns a colored status icon for a branch:
100108
// - ✓ (purple) if the PR has been merged
109+
// - ◎ (yellow) if the PR is queued in a merge queue
101110
// - ⚠ (yellow) if the branch needs rebasing (non-linear history)
102111
// - ○ (green) if there is an open PR
103112
func branchStatusIndicator(cfg *config.Config, s *stack.Stack, b stack.BranchRef) string {
104113
if b.IsMerged() {
105114
return " " + cfg.ColorMagenta("✓")
106115
}
107116

117+
if b.IsQueued() {
118+
return " " + cfg.ColorWarning("◎")
119+
}
120+
108121
baseBranch := s.ActiveBaseBranch(b.Branch)
109122
if needsRebase, err := git.IsAncestor(baseBranch, b.Branch); err == nil && !needsRebase {
110123
return " " + cfg.ColorWarning("⚠")
@@ -131,6 +144,7 @@ type viewJSONBranch struct {
131144
Base string `json:"base,omitempty"`
132145
IsCurrent bool `json:"isCurrent"`
133146
IsMerged bool `json:"isMerged"`
147+
IsQueued bool `json:"isQueued"`
134148
NeedsRebase bool `json:"needsRebase"`
135149
PR *viewJSONPR `json:"pr,omitempty"`
136150
}
@@ -156,6 +170,7 @@ func viewJSON(cfg *config.Config, s *stack.Stack, currentBranch string) error {
156170
Base: b.Base,
157171
IsCurrent: b.Branch == currentBranch,
158172
IsMerged: b.IsMerged(),
173+
IsQueued: b.IsQueued(),
159174
}
160175

161176
// Check if the branch needs rebasing (base not ancestor of branch).
@@ -170,6 +185,8 @@ func viewJSON(cfg *config.Config, s *stack.Stack, currentBranch string) error {
170185
state := "OPEN"
171186
if b.PullRequest.Merged {
172187
state = "MERGED"
188+
} else if b.IsQueued() {
189+
state = "QUEUED"
173190
}
174191
jb.PR = &viewJSONPR{
175192
Number: b.PullRequest.Number,

cmd/view_test.go

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88

99
"github.com/github/gh-stack/internal/config"
1010
"github.com/github/gh-stack/internal/git"
11+
"github.com/github/gh-stack/internal/github"
1112
"github.com/github/gh-stack/internal/stack"
1213
"github.com/stretchr/testify/assert"
1314
"github.com/stretchr/testify/require"
@@ -283,3 +284,135 @@ func TestViewShort_FullyMergedStack(t *testing.T) {
283284
assert.Contains(t, output, "b1")
284285
assert.Contains(t, output, "b2")
285286
}
287+
288+
// TestViewShort_QueuedStack verifies that --short output shows queued
289+
// branches with a "queued" separator and the ◎ icon.
290+
func TestViewShort_QueuedStack(t *testing.T) {
291+
s := stack.Stack{
292+
Trunk: stack.BranchRef{Branch: "main"},
293+
Branches: []stack.BranchRef{
294+
{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 1}},
295+
{Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 2}},
296+
{Branch: "b3", PullRequest: &stack.PullRequestRef{Number: 3}},
297+
},
298+
}
299+
300+
tmpDir := t.TempDir()
301+
writeStackFile(t, tmpDir, s)
302+
303+
restore := git.SetOps(&git.MockOps{
304+
GitDirFn: func() (string, error) { return tmpDir, nil },
305+
CurrentBranchFn: func() (string, error) { return "b3", nil },
306+
IsAncestorFn: func(string, string) (bool, error) { return true, nil },
307+
RevParseFn: func(ref string) (string, error) { return "sha-" + ref, nil },
308+
})
309+
defer restore()
310+
311+
// Mock GitHub client to return b1 as queued (MergeQueueEntry set)
312+
cfg, outR, _ := config.NewTestConfig()
313+
cfg.GitHubClientOverride = &github.MockClient{
314+
FindAnyPRForBranchFn: func(branch string) (*github.PullRequest, error) {
315+
switch branch {
316+
case "b1":
317+
return &github.PullRequest{
318+
Number: 1,
319+
ID: "PR_1",
320+
MergeQueueEntry: &github.MergeQueueEntry{ID: "MQE_1"},
321+
}, nil
322+
case "b2":
323+
return &github.PullRequest{Number: 2, ID: "PR_2"}, nil
324+
case "b3":
325+
return &github.PullRequest{Number: 3, ID: "PR_3"}, nil
326+
}
327+
return nil, nil
328+
},
329+
}
330+
331+
cmd := ViewCmd(cfg)
332+
cmd.SetArgs([]string{"--short"})
333+
cmd.SetOut(io.Discard)
334+
cmd.SetErr(io.Discard)
335+
err := cmd.Execute()
336+
337+
cfg.Out.Close()
338+
raw, _ := io.ReadAll(outR)
339+
output := string(raw)
340+
341+
assert.NoError(t, err)
342+
assert.Contains(t, output, "b1")
343+
assert.Contains(t, output, "b2")
344+
assert.Contains(t, output, "b3")
345+
assert.Contains(t, output, "queued", "should show queued separator")
346+
assert.Contains(t, output, "◎", "should show queued icon for b1")
347+
}
348+
349+
// TestViewShort_MixedQueuedAndMerged verifies that --short output shows
350+
// both "queued" and "merged" separators in the correct order.
351+
func TestViewShort_MixedQueuedAndMerged(t *testing.T) {
352+
s := stack.Stack{
353+
Trunk: stack.BranchRef{Branch: "main"},
354+
Branches: []stack.BranchRef{
355+
{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 1, Merged: true}},
356+
{Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 2}},
357+
{Branch: "b3", PullRequest: &stack.PullRequestRef{Number: 3}},
358+
},
359+
}
360+
361+
tmpDir := t.TempDir()
362+
writeStackFile(t, tmpDir, s)
363+
364+
restore := git.SetOps(&git.MockOps{
365+
GitDirFn: func() (string, error) { return tmpDir, nil },
366+
CurrentBranchFn: func() (string, error) { return "b3", nil },
367+
IsAncestorFn: func(string, string) (bool, error) { return true, nil },
368+
RevParseFn: func(ref string) (string, error) { return "sha-" + ref, nil },
369+
})
370+
defer restore()
371+
372+
// b1 is merged (persisted), b2 is queued (from API)
373+
cfg, outR, _ := config.NewTestConfig()
374+
cfg.GitHubClientOverride = &github.MockClient{
375+
FindAnyPRForBranchFn: func(branch string) (*github.PullRequest, error) {
376+
switch branch {
377+
case "b2":
378+
return &github.PullRequest{
379+
Number: 2,
380+
ID: "PR_2",
381+
MergeQueueEntry: &github.MergeQueueEntry{ID: "MQE_2"},
382+
}, nil
383+
case "b3":
384+
return &github.PullRequest{Number: 3, ID: "PR_3"}, nil
385+
}
386+
return nil, nil
387+
},
388+
}
389+
390+
cmd := ViewCmd(cfg)
391+
cmd.SetArgs([]string{"--short"})
392+
cmd.SetOut(io.Discard)
393+
cmd.SetErr(io.Discard)
394+
err := cmd.Execute()
395+
396+
cfg.Out.Close()
397+
raw, _ := io.ReadAll(outR)
398+
output := string(raw)
399+
400+
assert.NoError(t, err)
401+
assert.Contains(t, output, "queued", "should show queued separator")
402+
assert.Contains(t, output, "merged", "should show merged separator")
403+
404+
// "merged" section (b1) should appear below "queued" section (b2) in output
405+
// Since we render top-to-bottom: b3 (active) -> queued separator -> b2 -> merged separator -> b1
406+
queuedIdx := indexOf(output, "queued")
407+
mergedIdx := indexOf(output, "merged")
408+
assert.Less(t, queuedIdx, mergedIdx, "queued separator should appear before merged separator")
409+
}
410+
411+
func indexOf(s, substr string) int {
412+
for i := 0; i <= len(s)-len(substr); i++ {
413+
if s[i:i+len(substr)] == substr {
414+
return i
415+
}
416+
}
417+
return -1
418+
}

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ require (
1313
github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d
1414
github.com/spf13/cobra v1.10.2
1515
github.com/stretchr/testify v1.11.1
16+
golang.org/x/sys v0.39.0
1617
golang.org/x/text v0.32.0
1718
)
1819

@@ -45,7 +46,6 @@ require (
4546
github.com/spf13/pflag v1.0.10 // indirect
4647
github.com/thlib/go-timezone-local v0.0.6 // indirect
4748
github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect
48-
golang.org/x/sys v0.39.0 // indirect
4949
golang.org/x/term v0.38.0 // indirect
5050
gopkg.in/yaml.v3 v3.0.1 // indirect
5151
)

0 commit comments

Comments
 (0)