Skip to content

Commit

Permalink
Find PRs using @{push}
Browse files Browse the repository at this point in the history
When using a triangular workflow with push.default = current and
upstream being set to the target branch [1], or a similar but simpler
central workflow [2], we should use @{push} instead to locate the remote
branch.

In fact, @{push} covers most cases in push.default = upstream too. The
branch.<name>.merge is probably only needed when using RemoteURL and
different remote / local branch names.

[1] cli#575 (comment)
[2] tpope/vim-fugitive#1172 (comment)
  • Loading branch information
Frederick888 committed Jun 14, 2024
1 parent 5e7ba54 commit 9bee0d8
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 2 deletions.
3 changes: 3 additions & 0 deletions git/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,9 @@ func (c *Client) ReadBranchConfig(ctx context.Context, branch string) (cfg Branc
cfg.MergeRef = parts[1]
}
}
if out, err = c.revParse(ctx, "--verify", "--quiet", "--abbrev-ref", branch+"@{push}"); err == nil {
cfg.Push = strings.TrimSuffix(string(out), "\n")
}
return
}

Expand Down
1 change: 1 addition & 0 deletions git/objects.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,5 @@ type BranchConfig struct {
RemoteName string
RemoteURL *url.URL
MergeRef string
Push string
}
4 changes: 3 additions & 1 deletion pkg/cmd/pr/shared/finder.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,9 @@ func (f *finder) parseCurrentBranch() (string, int, error) {
}

if branchOwner != "" {
if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
if branchConfig.Push != "" {
prHeadRef = strings.TrimPrefix(branchConfig.Push, branchConfig.RemoteName+"/")
} else if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
prHeadRef = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/")
}
// prepend `OWNER:` if this branch is pushed to a fork
Expand Down
10 changes: 10 additions & 0 deletions pkg/cmd/pr/shared/finder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,17 @@ func TestFind(t *testing.T) {
return "blueberries", nil
},
branchConfig: func(branch string) (c git.BranchConfig) {
c.MergeRef = "refs/heads/blueberries"
c.RemoteName = "origin"
c.Push = "origin/blueberries"
return
},
remotesFn: func() (context.Remotes, error) {
return context.Remotes{{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.New("OWNER", "REPO"),
}}, nil
},
},
httpStub: func(r *httpmock.Registry) {
r.Register(
Expand Down Expand Up @@ -318,6 +327,7 @@ func TestFind(t *testing.T) {
branchConfig: func(branch string) (c git.BranchConfig) {
c.MergeRef = "refs/heads/blue-upstream-berries"
c.RemoteName = "origin"
c.Push = "origin/blue-upstream-berries"
return
},
remotesFn: func() (context.Remotes, error) {
Expand Down
4 changes: 3 additions & 1 deletion pkg/cmd/pr/status/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,9 @@ func prSelectorForCurrentBranch(gitClient *git.Client, baseRepo ghrepo.Interface
}

if branchOwner != "" {
if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
if branchConfig.Push != "" {
selector = strings.TrimPrefix(branchConfig.Push, branchConfig.RemoteName+"/")
} else if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
selector = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/")
}
// prepend `OWNER:` if this branch is pushed to a fork
Expand Down

0 comments on commit 9bee0d8

Please sign in to comment.