STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Cleaned up some naming and comments

Browse source
This commit is contained in:
Tyler McGoffin 2025-01-27 16:40:47 -08:00
parent 62106dc800
commit e31bfd0092
5 changed files with 37 additions and 32 deletions
Download patch file Download diff file Expand all files Collapse all files

20
git/client.go
View file

@ -389,13 +389,11 @@ func (c *Client) ReadBranchConfig(ctx context.Context, branch string) (BranchCon
return BranchConfig{}, err
}
// This is the error we expect if a git command does not run successfully.
// If the ExitCode is 1, then we just didn't find any config for the branch.
// We will use this error to check against the commands that are allowed to
// return an empty result.
var gitError *GitError
branchCfgOut, err := cmd.Output()
if err != nil {
// This is the error we expect if the git command does not run successfully.
// If the ExitCode is 1, then we just didn't find any config for the branch.
var gitError *GitError
if ok := errors.As(err, &gitError); ok && gitError.ExitCode != 1 {
return BranchConfig{}, err
}
@ -417,14 +415,9 @@ func parseBranchConfig(branchConfigLines []string) BranchConfig {
keys := strings.Split(parts[0], ".")
switch keys[len(keys)-1] {
case "remote":
remoteURL, remoteName := parseRemoteURLOrName(parts[1])
cfg.RemoteURL = remoteURL
cfg.RemoteName = remoteName
// pushremote usually indicates a "triangular" workflow
cfg.RemoteURL, cfg.RemoteName = parseRemoteURLOrName(parts[1])
case "pushremote":
pushRemoteURL, pushRemoteName := parseRemoteURLOrName(parts[1])
cfg.PushRemoteURL = pushRemoteURL
cfg.PushRemoteName = pushRemoteName
cfg.PushRemoteURL, cfg.PushRemoteName = parseRemoteURLOrName(parts[1])
case "merge":
cfg.MergeRef = parts[1]
case MergeBaseConfig:
@ -449,7 +442,8 @@ func (c *Client) SetBranchConfig(ctx context.Context, branch, name, value string
}
// PushDefault returns the value of push.default in the config. If the value
// is not set, it returns "simple" (the default value).
// is not set, it returns "simple" (the default git value). See
// https://git-scm.com/docs/git-config#Documentation/git-config.txt-pushdefault
func (c *Client) PushDefault(ctx context.Context) (string, error) {
pushDefault, err := c.Config(ctx, "push.default")
if err == nil {

20
git/client_test.go
View file

@ -14,6 +14,7 @@ import (
"strings"
"testing"
"github.com/MakeNowJust/heredoc"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@ -763,7 +764,12 @@ func TestClientReadBranchConfig(t *testing.T) {
name: "when the config is read, it should return the correct BranchConfig",
cmds: mockedCommands{
`path/to/git config --get-regexp ^branch\.trunk\.(remote|merge|pushremote|gh-merge-base)$`: {
Stdout: "branch.trunk.remote upstream\nbranch.trunk.merge refs/heads/trunk\nbranch.trunk.pushremote origin\nbranch.trunk.gh-merge-base gh-merge-base\n",
Stdout: heredoc.Doc(`
branch.trunk.remote upstream
branch.trunk.merge refs/heads/trunk
branch.trunk.pushremote origin
branch.trunk.gh-merge-base gh-merge-base
`),
},
},
branch: "trunk",
@ -1869,16 +1875,16 @@ func TestCommandMocking(t *testing.T) {
jsonVar, ok := os.LookupEnv("GH_HELPER_PROCESS_RICH_COMMANDS")
if !ok {
fmt.Fprint(os.Stderr, "missing GH_HELPER_PROCESS_RICH_COMMANDS")
// Exit 1 is reserved for empty command responses by git. This can be desirable in some cases,
// so returning an arbitrary exit code to avoid suppressing this if an exit code 1 is allowed.
// Exit 1 is used for empty key values in the git config. This is non-breaking in those use cases,
// so this is returning a non-zero exit code to avoid suppressing this error for those use cases.
os.Exit(16)
}
var commands mockedCommands
if err := json.Unmarshal([]byte(jsonVar), &commands); err != nil {
fmt.Fprint(os.Stderr, "failed to unmarshal GH_HELPER_PROCESS_RICH_COMMANDS")
// Exit 1 is reserved for empty command responses by git. This can be desirable in some cases,
// so returning an arbitrary exit code to avoid suppressing this if an exit code 1 is allowed.
// Exit 1 is used for empty key values in the git config. This is non-breaking in those use cases,
// so this is returning a non-zero exit code to avoid suppressing this error for those use cases.
os.Exit(16)
}
@ -1888,8 +1894,8 @@ func TestCommandMocking(t *testing.T) {
commandResult, ok := commands[args(strings.Join(realArgs, " "))]
if !ok {
fmt.Fprintf(os.Stderr, "unexpected command: %s\n", strings.Join(realArgs, " "))
// Exit 1 is reserved for empty command responses by git. This can be desirable in some cases,
// so returning an arbitrary exit code to avoid suppressing this if an exit code 1 is allowed.
// Exit 1 is used for empty key values in the git config. This is non-breaking in those use cases,
// so this is returning a non-zero exit code to avoid suppressing this error for those use cases.
os.Exit(16)
}

13
git/objects.go
View file

@ -60,13 +60,14 @@ type Commit struct {
Body string
}
// These are the keys we read from the git branch.<name> config.
type BranchConfig struct {
RemoteName string
RemoteURL *url.URL
RemoteName string // .remote if string
RemoteURL *url.URL // .remote if url
MergeRef string // .merge
PushRemoteName string // .pushremote if string
PushRemoteURL *url.URL // .pushremote if url
// MergeBase is the optional base branch to target in a new PR if `--base` is not specified.
MergeBase string
MergeRef string
// These are usually used to handle triangular workflows.
PushRemoteURL *url.URL
PushRemoteName string
}

6
pkg/cmd/pr/shared/finder.go
View file

@ -109,10 +109,10 @@ type PRRefs struct {
BaseRepo ghrepo.Interface
}
// GetPRLabel returns the string that the GitHub API uses to identify the PR. This is
// GetPRHeadLabel returns the string that the GitHub API uses to identify the PR. This is
// either just the branch name or, if the PR is originating from a fork, the fork owner
// and the branch name, like <owner>:<branch>.
func (s *PRRefs) GetPRLabel() string {
func (s *PRRefs) GetPRHeadLabel() string {
if ghrepo.IsSame(s.HeadRepo, s.BaseRepo) {
return s.BranchName
}
@ -240,7 +240,7 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err
return nil, nil, err
}
pr, err = findForBranch(httpClient, f.baseRefRepo, opts.BaseBranch, prRefs.GetPRLabel(), opts.States, fields.ToSlice())
pr, err = findForBranch(httpClient, f.baseRefRepo, opts.BaseBranch, prRefs.GetPRHeadLabel(), opts.States, fields.ToSlice())
if err != nil {
return pr, f.baseRefRepo, err
}

10
pkg/cmd/pr/shared/finder_test.go
View file

@ -419,7 +419,11 @@ func TestFind(t *testing.T) {
wantRepo: "https://github.com/ORIGINOWNER/REPO",
},
{
name: "the current branch is configured to push to and pull from a URL (upstream, in this example) that is different from what the repo is configured to push to and pull from (origin, in this example) and push.default = upstream, it finds the PR associated with the upstream repo and returns origin as the base repo",
// The current BRANCH is configured to push to and pull from a URL (upstream, in this example)
// which is different from what the REPO is configured to push to and pull from (origin, in this example)
// and push.default = upstream. It should find the PR associated with the upstream repo and return
// origin as the base repo
name: "when push.default = upstream and the current branch is configured to push/pull from a different remote than the repo",
args: args{
selector: "",
fields: []string{"id", "number"},
@ -902,7 +906,7 @@ func TestParsePRRefs(t *testing.T) {
}
}
func TestPRRefs_GetPRLabel(t *testing.T) {
func TestPRRefs_GetPRHeadLabel(t *testing.T) {
originRepo := ghrepo.New("ORIGINOWNER", "REPO")
upstreamRepo := ghrepo.New("UPSTREAMOWNER", "REPO")
tests := []struct {
@ -931,7 +935,7 @@ func TestPRRefs_GetPRLabel(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, tt.prRefs.GetPRLabel())
assert.Equal(t, tt.want, tt.prRefs.GetPRHeadLabel())
})
}
}