Remove named returns from ReadBranchConfig and surface errors

This commit is contained in:
Tyler McGoffin 2025-01-07 11:31:46 -08:00
parent 40984d1eb6
commit bf6fdbdfd2
6 changed files with 20 additions and 9 deletions

View file

@ -377,16 +377,22 @@ func (c *Client) lookupCommit(ctx context.Context, sha, format string) ([]byte,
}
// ReadBranchConfig parses the `branch.BRANCH.(remote|merge|gh-merge-base)` part of git config.
func (c *Client) ReadBranchConfig(ctx context.Context, branch string) (cfg BranchConfig) {
// If no branch config is found or there is an error in the command, it returns an empty BranchConfig.
// Downstream consumers of ReadBranchConfig should consider the behavior they desire if this errors,
// as an empty config is not necessarily breaking.
func (c *Client) ReadBranchConfig(ctx context.Context, branch string) (BranchConfig, error) {
var cfg BranchConfig
prefix := regexp.QuoteMeta(fmt.Sprintf("branch.%s.", branch))
args := []string{"config", "--get-regexp", fmt.Sprintf("^%s(remote|merge|%s)$", prefix, MergeBaseConfig)}
cmd, err := c.Command(ctx, args...)
if err != nil {
return
return cfg, err
}
out, err := cmd.Output()
if err != nil {
return
return cfg, err
}
for _, line := range outputLines(out) {
@ -412,7 +418,7 @@ func (c *Client) ReadBranchConfig(ctx context.Context, branch string) (cfg Branc
cfg.MergeBase = parts[1]
}
}
return
return cfg, nil
}
// SetBranchConfig sets the named value on the given branch.

View file

@ -747,7 +747,7 @@ func TestClientReadBranchConfig(t *testing.T) {
GitPath: "path/to/git",
commandContext: cmdCtx,
}
branchConfig := client.ReadBranchConfig(context.Background(), "trunk")
branchConfig, _ := client.ReadBranchConfig(context.Background(), "trunk")
assert.Equal(t, tt.wantCmdArgs, strings.Join(cmd.Args[3:], " "))
assert.Equal(t, tt.wantBranchConfig, branchConfig)
})

View file

@ -680,7 +680,11 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) {
var headRepo ghrepo.Interface
var headRemote *ghContext.Remote
headBranchConfig := gitClient.ReadBranchConfig(context.Background(), headBranch)
headBranchConfig, err := gitClient.ReadBranchConfig(context.Background(), headBranch)
if err != nil {
// We can still proceed without the branch config, so print to stderr but continue
fmt.Fprintf(opts.IO.ErrOut, "Error reading git branch config: %v\n", err)
}
if isPushEnabled {
// determine whether the head branch is already pushed to a remote
if trackingRef, found := tryDetermineTrackingRef(gitClient, remotes, headBranch, headBranchConfig); found {

View file

@ -1717,7 +1717,7 @@ func Test_tryDetermineTrackingRef(t *testing.T) {
GhPath: "some/path/gh",
GitPath: "some/path/git",
}
headBranchConfig := gitClient.ReadBranchConfig(ctx.Background(), "feature")
headBranchConfig, _ := gitClient.ReadBranchConfig(ctx.Background(), "feature")
ref, found := tryDetermineTrackingRef(gitClient, tt.remotes, "feature", headBranchConfig)
assert.Equal(t, tt.expectedTrackingRef, ref)

View file

@ -59,7 +59,8 @@ func NewFinder(factory *cmdutil.Factory) PRFinder {
httpClient: factory.HttpClient,
progress: factory.IOStreams,
branchConfig: func(s string) git.BranchConfig {
return factory.GitClient.ReadBranchConfig(context.Background(), s)
branchConfig, _ := factory.GitClient.ReadBranchConfig(context.Background(), s)
return branchConfig
},
}
}

View file

@ -186,7 +186,7 @@ func statusRun(opts *StatusOptions) error {
func prSelectorForCurrentBranch(gitClient *git.Client, baseRepo ghrepo.Interface, prHeadRef string, rem ghContext.Remotes) (prNumber int, selector string, err error) {
selector = prHeadRef
branchConfig := gitClient.ReadBranchConfig(context.Background(), prHeadRef)
branchConfig, _ := gitClient.ReadBranchConfig(context.Background(), prHeadRef)
// the branch is configured to merge a special PR head ref
prHeadRE := regexp.MustCompile(`^refs/pull/(\d+)/head$`)