From 36397dbdc5defd12f3eaeb21307d592ed91d300d Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 4 Feb 2020 13:18:05 -0600 Subject: [PATCH 01/51] move checkout to its own file --- command/pr.go | 103 ------------------------------------- command/pr_checkout.go | 113 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 103 deletions(-) create mode 100644 command/pr_checkout.go diff --git a/command/pr.go b/command/pr.go index 624debac0..38ee25c4e 100644 --- a/command/pr.go +++ b/command/pr.go @@ -1,11 +1,8 @@ package command import ( - "errors" "fmt" "io" - "os" - "os/exec" "regexp" "strconv" "strings" @@ -46,17 +43,6 @@ A pull request can be supplied as argument in any of the following formats: - by URL, e.g. "https://github.com/OWNER/REPO/pull/123"; or - by the name of its head branch, e.g. "patch-1" or "OWNER:patch-1".`, } -var prCheckoutCmd = &cobra.Command{ - Use: "checkout { | | }", - Short: "Check out a pull request in Git", - Args: func(cmd *cobra.Command, args []string) error { - if len(args) < 1 { - return errors.New("requires a PR number as an argument") - } - return nil - }, - RunE: prCheckout, -} var prListCmd = &cobra.Command{ Use: "list", Short: "List and filter pull requests in this repository", @@ -386,95 +372,6 @@ func prSelectorForCurrentBranch(ctx context.Context) (prNumber int, prHeadRef st return } -func prCheckout(cmd *cobra.Command, args []string) error { - ctx := contextForCommand(cmd) - currentBranch, _ := ctx.Branch() - remotes, err := ctx.Remotes() - if err != nil { - return err - } - // FIXME: duplicates logic from fsContext.BaseRepo - baseRemote, err := remotes.FindByName("upstream", "github", "origin", "*") - if err != nil { - return err - } - apiClient, err := apiClientForContext(ctx) - if err != nil { - return err - } - - pr, err := prFromArg(apiClient, baseRemote, args[0]) - if err != nil { - return err - } - - headRemote := baseRemote - if pr.IsCrossRepository { - headRemote, _ = remotes.FindByRepo(pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name) - } - - cmdQueue := [][]string{} - - newBranchName := pr.HeadRefName - if headRemote != nil { - // there is an existing git remote for PR head - remoteBranch := fmt.Sprintf("%s/%s", headRemote.Name, pr.HeadRefName) - refSpec := fmt.Sprintf("+refs/heads/%s:refs/remotes/%s", pr.HeadRefName, remoteBranch) - - cmdQueue = append(cmdQueue, []string{"git", "fetch", headRemote.Name, refSpec}) - - // local branch already exists - if git.VerifyRef("refs/heads/" + newBranchName) { - cmdQueue = append(cmdQueue, []string{"git", "checkout", newBranchName}) - cmdQueue = append(cmdQueue, []string{"git", "merge", "--ff-only", fmt.Sprintf("refs/remotes/%s", remoteBranch)}) - } else { - cmdQueue = append(cmdQueue, []string{"git", "checkout", "-b", newBranchName, "--no-track", remoteBranch}) - cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.remote", newBranchName), headRemote.Name}) - cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.merge", newBranchName), "refs/heads/" + pr.HeadRefName}) - } - } else { - // no git remote for PR head - - // avoid naming the new branch the same as the default branch - if newBranchName == pr.HeadRepository.DefaultBranchRef.Name { - newBranchName = fmt.Sprintf("%s/%s", pr.HeadRepositoryOwner.Login, newBranchName) - } - - ref := fmt.Sprintf("refs/pull/%d/head", pr.Number) - if newBranchName == currentBranch { - // PR head matches currently checked out branch - cmdQueue = append(cmdQueue, []string{"git", "fetch", baseRemote.Name, ref}) - cmdQueue = append(cmdQueue, []string{"git", "merge", "--ff-only", "FETCH_HEAD"}) - } else { - // create a new branch - cmdQueue = append(cmdQueue, []string{"git", "fetch", baseRemote.Name, fmt.Sprintf("%s:%s", ref, newBranchName)}) - cmdQueue = append(cmdQueue, []string{"git", "checkout", newBranchName}) - } - - remote := baseRemote.Name - mergeRef := ref - if pr.MaintainerCanModify { - remote = fmt.Sprintf("https://github.com/%s/%s.git", pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name) - mergeRef = fmt.Sprintf("refs/heads/%s", pr.HeadRefName) - } - if mc, err := git.Config(fmt.Sprintf("branch.%s.merge", newBranchName)); err != nil || mc == "" { - cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.remote", newBranchName), remote}) - cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.merge", newBranchName), mergeRef}) - } - } - - for _, args := range cmdQueue { - cmd := exec.Command(args[0], args[1:]...) - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - if err := utils.PrepareCmd(cmd).Run(); err != nil { - return err - } - } - - return nil -} - func printPrs(w io.Writer, totalCount int, prs ...api.PullRequest) { for _, pr := range prs { prNumber := fmt.Sprintf("#%d", pr.Number) diff --git a/command/pr_checkout.go b/command/pr_checkout.go new file mode 100644 index 000000000..43556c67f --- /dev/null +++ b/command/pr_checkout.go @@ -0,0 +1,113 @@ +package command + +import ( + "errors" + "fmt" + "os" + "os/exec" + + "github.com/cli/cli/git" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +func prCheckout(cmd *cobra.Command, args []string) error { + ctx := contextForCommand(cmd) + currentBranch, _ := ctx.Branch() + remotes, err := ctx.Remotes() + if err != nil { + return err + } + // FIXME: duplicates logic from fsContext.BaseRepo + baseRemote, err := remotes.FindByName("upstream", "github", "origin", "*") + if err != nil { + return err + } + apiClient, err := apiClientForContext(ctx) + if err != nil { + return err + } + + pr, err := prFromArg(apiClient, baseRemote, args[0]) + if err != nil { + return err + } + + headRemote := baseRemote + if pr.IsCrossRepository { + headRemote, _ = remotes.FindByRepo(pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name) + } + + cmdQueue := [][]string{} + + newBranchName := pr.HeadRefName + if headRemote != nil { + // there is an existing git remote for PR head + remoteBranch := fmt.Sprintf("%s/%s", headRemote.Name, pr.HeadRefName) + refSpec := fmt.Sprintf("+refs/heads/%s:refs/remotes/%s", pr.HeadRefName, remoteBranch) + + cmdQueue = append(cmdQueue, []string{"git", "fetch", headRemote.Name, refSpec}) + + // local branch already exists + if git.VerifyRef("refs/heads/" + newBranchName) { + cmdQueue = append(cmdQueue, []string{"git", "checkout", newBranchName}) + cmdQueue = append(cmdQueue, []string{"git", "merge", "--ff-only", fmt.Sprintf("refs/remotes/%s", remoteBranch)}) + } else { + cmdQueue = append(cmdQueue, []string{"git", "checkout", "-b", newBranchName, "--no-track", remoteBranch}) + cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.remote", newBranchName), headRemote.Name}) + cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.merge", newBranchName), "refs/heads/" + pr.HeadRefName}) + } + } else { + // no git remote for PR head + + // avoid naming the new branch the same as the default branch + if newBranchName == pr.HeadRepository.DefaultBranchRef.Name { + newBranchName = fmt.Sprintf("%s/%s", pr.HeadRepositoryOwner.Login, newBranchName) + } + + ref := fmt.Sprintf("refs/pull/%d/head", pr.Number) + if newBranchName == currentBranch { + // PR head matches currently checked out branch + cmdQueue = append(cmdQueue, []string{"git", "fetch", baseRemote.Name, ref}) + cmdQueue = append(cmdQueue, []string{"git", "merge", "--ff-only", "FETCH_HEAD"}) + } else { + // create a new branch + cmdQueue = append(cmdQueue, []string{"git", "fetch", baseRemote.Name, fmt.Sprintf("%s:%s", ref, newBranchName)}) + cmdQueue = append(cmdQueue, []string{"git", "checkout", newBranchName}) + } + + remote := baseRemote.Name + mergeRef := ref + if pr.MaintainerCanModify { + remote = fmt.Sprintf("https://github.com/%s/%s.git", pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name) + mergeRef = fmt.Sprintf("refs/heads/%s", pr.HeadRefName) + } + if mc, err := git.Config(fmt.Sprintf("branch.%s.merge", newBranchName)); err != nil || mc == "" { + cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.remote", newBranchName), remote}) + cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.merge", newBranchName), mergeRef}) + } + } + + for _, args := range cmdQueue { + cmd := exec.Command(args[0], args[1:]...) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + if err := utils.PrepareCmd(cmd).Run(); err != nil { + return err + } + } + + return nil +} + +var prCheckoutCmd = &cobra.Command{ + Use: "checkout { | | }", + Short: "Check out a pull request in Git", + Args: func(cmd *cobra.Command, args []string) error { + if len(args) < 1 { + return errors.New("requires a PR number as an argument") + } + return nil + }, + RunE: prCheckout, +} From 5b8d7c1841099612d94129cbc1e81505d3ad778d Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 4 Feb 2020 17:49:02 -0600 Subject: [PATCH 02/51] prefix branch names for local pr checkout --- command/pr_checkout.go | 3 +- command/pr_checkout_test.go | 58 ++++++++++++++++++------------------- 2 files changed, 31 insertions(+), 30 deletions(-) diff --git a/command/pr_checkout.go b/command/pr_checkout.go index 43556c67f..0c41dc793 100644 --- a/command/pr_checkout.go +++ b/command/pr_checkout.go @@ -40,7 +40,8 @@ func prCheckout(cmd *cobra.Command, args []string) error { cmdQueue := [][]string{} - newBranchName := pr.HeadRefName + // namespace PR checkout branches to avoid local branch name collisions + newBranchName := fmt.Sprintf("pr/%d/%s", pr.Number, pr.HeadRefName) if headRemote != nil { // there is an existing git remote for PR head remoteBranch := fmt.Sprintf("%s/%s", headRemote.Name, pr.HeadRefName) diff --git a/command/pr_checkout_test.go b/command/pr_checkout_test.go index ba0ddff22..9500d4530 100644 --- a/command/pr_checkout_test.go +++ b/command/pr_checkout_test.go @@ -42,7 +42,7 @@ func TestPRCheckout_sameRepo(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { switch strings.Join(cmd.Args, " ") { - case "git show-ref --verify --quiet refs/heads/feature": + case "git show-ref --verify --quiet refs/heads/pr/123/feature": return &errorStub{"exit status: 1"} default: ranCommands = append(ranCommands, cmd.Args) @@ -57,9 +57,9 @@ func TestPRCheckout_sameRepo(t *testing.T) { eq(t, len(ranCommands), 4) eq(t, strings.Join(ranCommands[0], " "), "git fetch origin +refs/heads/feature:refs/remotes/origin/feature") - eq(t, strings.Join(ranCommands[1], " "), "git checkout -b feature --no-track origin/feature") - eq(t, strings.Join(ranCommands[2], " "), "git config branch.feature.remote origin") - eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.merge refs/heads/feature") + eq(t, strings.Join(ranCommands[1], " "), "git checkout -b pr/123/feature --no-track origin/feature") + eq(t, strings.Join(ranCommands[2], " "), "git config branch.pr/123/feature.remote origin") + eq(t, strings.Join(ranCommands[3], " "), "git config branch.pr/123/feature.merge refs/heads/feature") } func TestPRCheckout_urlArg(t *testing.T) { @@ -94,7 +94,7 @@ func TestPRCheckout_urlArg(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { switch strings.Join(cmd.Args, " ") { - case "git show-ref --verify --quiet refs/heads/feature": + case "git show-ref --verify --quiet refs/heads/pr/123/feature": return &errorStub{"exit status: 1"} default: ranCommands = append(ranCommands, cmd.Args) @@ -108,7 +108,7 @@ func TestPRCheckout_urlArg(t *testing.T) { eq(t, output.String(), "") eq(t, len(ranCommands), 4) - eq(t, strings.Join(ranCommands[1], " "), "git checkout -b feature --no-track origin/feature") + eq(t, strings.Join(ranCommands[1], " "), "git checkout -b pr/123/feature --no-track origin/feature") } func TestPRCheckout_branchArg(t *testing.T) { @@ -143,7 +143,7 @@ func TestPRCheckout_branchArg(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { switch strings.Join(cmd.Args, " ") { - case "git show-ref --verify --quiet refs/heads/feature": + case "git show-ref --verify --quiet refs/heads/pr/123/feature": return &errorStub{"exit status: 1"} default: ranCommands = append(ranCommands, cmd.Args) @@ -157,7 +157,7 @@ func TestPRCheckout_branchArg(t *testing.T) { eq(t, output.String(), "") eq(t, len(ranCommands), 5) - eq(t, strings.Join(ranCommands[1], " "), "git fetch origin refs/pull/123/head:feature") + eq(t, strings.Join(ranCommands[1], " "), "git fetch origin refs/pull/123/head:pr/123/feature") } func TestPRCheckout_existingBranch(t *testing.T) { @@ -192,7 +192,7 @@ func TestPRCheckout_existingBranch(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { switch strings.Join(cmd.Args, " ") { - case "git show-ref --verify --quiet refs/heads/feature": + case "git show-ref --verify --quiet refs/heads/pr/123/feature": return &outputStub{} default: ranCommands = append(ranCommands, cmd.Args) @@ -207,7 +207,7 @@ func TestPRCheckout_existingBranch(t *testing.T) { eq(t, len(ranCommands), 3) eq(t, strings.Join(ranCommands[0], " "), "git fetch origin +refs/heads/feature:refs/remotes/origin/feature") - eq(t, strings.Join(ranCommands[1], " "), "git checkout feature") + eq(t, strings.Join(ranCommands[1], " "), "git checkout pr/123/feature") eq(t, strings.Join(ranCommands[2], " "), "git merge --ff-only refs/remotes/origin/feature") } @@ -244,7 +244,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { switch strings.Join(cmd.Args, " ") { - case "git show-ref --verify --quiet refs/heads/feature": + case "git show-ref --verify --quiet refs/heads/pr/123/feature": return &errorStub{"exit status: 1"} default: ranCommands = append(ranCommands, cmd.Args) @@ -259,9 +259,9 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) { eq(t, len(ranCommands), 4) eq(t, strings.Join(ranCommands[0], " "), "git fetch robot-fork +refs/heads/feature:refs/remotes/robot-fork/feature") - eq(t, strings.Join(ranCommands[1], " "), "git checkout -b feature --no-track robot-fork/feature") - eq(t, strings.Join(ranCommands[2], " "), "git config branch.feature.remote robot-fork") - eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.merge refs/heads/feature") + eq(t, strings.Join(ranCommands[1], " "), "git checkout -b pr/123/feature --no-track robot-fork/feature") + eq(t, strings.Join(ranCommands[2], " "), "git config branch.pr/123/feature.remote robot-fork") + eq(t, strings.Join(ranCommands[3], " "), "git config branch.pr/123/feature.merge refs/heads/feature") } func TestPRCheckout_differentRepo(t *testing.T) { @@ -296,7 +296,7 @@ func TestPRCheckout_differentRepo(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { switch strings.Join(cmd.Args, " ") { - case "git config branch.feature.merge": + case "git config branch.pr/123/feature.merge": return &errorStub{"exit status 1"} default: ranCommands = append(ranCommands, cmd.Args) @@ -310,10 +310,10 @@ func TestPRCheckout_differentRepo(t *testing.T) { eq(t, output.String(), "") eq(t, len(ranCommands), 4) - eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:feature") - eq(t, strings.Join(ranCommands[1], " "), "git checkout feature") - eq(t, strings.Join(ranCommands[2], " "), "git config branch.feature.remote origin") - eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.merge refs/pull/123/head") + eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:pr/123/feature") + eq(t, strings.Join(ranCommands[1], " "), "git checkout pr/123/feature") + eq(t, strings.Join(ranCommands[2], " "), "git config branch.pr/123/feature.remote origin") + eq(t, strings.Join(ranCommands[3], " "), "git config branch.pr/123/feature.merge refs/pull/123/head") } func TestPRCheckout_differentRepo_existingBranch(t *testing.T) { @@ -348,7 +348,7 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { switch strings.Join(cmd.Args, " ") { - case "git config branch.feature.merge": + case "git config branch.pr/123/feature.merge": return &outputStub{[]byte("refs/heads/feature\n")} default: ranCommands = append(ranCommands, cmd.Args) @@ -362,13 +362,13 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) { eq(t, output.String(), "") eq(t, len(ranCommands), 2) - eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:feature") - eq(t, strings.Join(ranCommands[1], " "), "git checkout feature") + eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:pr/123/feature") + eq(t, strings.Join(ranCommands[1], " "), "git checkout pr/123/feature") } func TestPRCheckout_differentRepo_currentBranch(t *testing.T) { ctx := context.NewBlank() - ctx.SetBranch("feature") + ctx.SetBranch("pr/123/feature") ctx.SetRemotes(map[string]string{ "origin": "OWNER/REPO", }) @@ -398,7 +398,7 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { switch strings.Join(cmd.Args, " ") { - case "git config branch.feature.merge": + case "git config branch.pr/123/feature.merge": return &outputStub{[]byte("refs/heads/feature\n")} default: ranCommands = append(ranCommands, cmd.Args) @@ -448,7 +448,7 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { switch strings.Join(cmd.Args, " ") { - case "git config branch.feature.merge": + case "git config branch.pr/123/feature.merge": return &errorStub{"exit status 1"} default: ranCommands = append(ranCommands, cmd.Args) @@ -462,8 +462,8 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) { eq(t, output.String(), "") eq(t, len(ranCommands), 4) - eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:feature") - eq(t, strings.Join(ranCommands[1], " "), "git checkout feature") - eq(t, strings.Join(ranCommands[2], " "), "git config branch.feature.remote https://github.com/hubot/REPO.git") - eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.merge refs/heads/feature") + eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:pr/123/feature") + eq(t, strings.Join(ranCommands[1], " "), "git checkout pr/123/feature") + eq(t, strings.Join(ranCommands[2], " "), "git config branch.pr/123/feature.remote https://github.com/hubot/REPO.git") + eq(t, strings.Join(ranCommands[3], " "), "git config branch.pr/123/feature.merge refs/heads/feature") } From 656158950d403f571c89ea80443cf5cbe7491872 Mon Sep 17 00:00:00 2001 From: OscarDOM Date: Wed, 5 Feb 2020 01:08:46 +0100 Subject: [PATCH 03/51] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5a19f287c..f5de324cd 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ And if you spot bugs or have features that you'd really like to see in `gh`, ple - `gh issue [status, list, view, create]` - `gh help` -Check out the [docs][] for more information. +Check out the [docs][https://cli.github.io/cli/] for more information. ## Comparison with hub From 64f94367ce16af38c601a803090a2fc04bf2974b Mon Sep 17 00:00:00 2001 From: Tiernan L Date: Tue, 4 Feb 2020 14:11:39 -1000 Subject: [PATCH 04/51] Swapping to this link https://cli.github.io/cli/manual/gh --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f5de324cd..f418aecf7 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ And if you spot bugs or have features that you'd really like to see in `gh`, ple - `gh issue [status, list, view, create]` - `gh help` -Check out the [docs][https://cli.github.io/cli/] for more information. +Check out the [docs][https://cli.github.io/cli/manual/gh] for more information. ## Comparison with hub From 163bc43bc136c41e69845e20748511dc53c3c8ad Mon Sep 17 00:00:00 2001 From: OscarDOM Date: Wed, 5 Feb 2020 01:31:14 +0100 Subject: [PATCH 05/51] Update README.md --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index f418aecf7..cd3d44dc2 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ And if you spot bugs or have features that you'd really like to see in `gh`, ple - `gh issue [status, list, view, create]` - `gh help` -Check out the [docs][https://cli.github.io/cli/manual/gh] for more information. +Check out the [docs][] for more information. ## Comparison with hub @@ -74,7 +74,7 @@ Install a prebuilt binary from the [releases page][] ### [Build from source](/source.md) -[docs]: https://cli.github.io/cli/gh +[docs]: https://cli.github.io/cli/manual/gh [scoop]: https://scoop.sh [releases page]: https://github.com/cli/cli/releases/latest [hub]: https://github.com/github/hub From 1205ad91ebab2ac4a054f1f4080a92b6db96ccd9 Mon Sep 17 00:00:00 2001 From: sh0rez Date: Thu, 6 Feb 2020 13:38:26 +0100 Subject: [PATCH 06/51] doc: add arch install instructions --- README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/README.md b/README.md index cd3d44dc2..df22d122b 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,14 @@ Install and upgrade: 1. Download the `.rpm` file from the [releases page][] 2. `sudo yum localinstall gh_*_linux_amd64.rpm` install the downloaded file +### Arch Linux + +Arch Linux users can install from the AUR: https://aur.archlinux.org/packages/gh/ + +```bash +$ yay -S gh +``` + ### Other platforms Install a prebuilt binary from the [releases page][] From 0da478d5d74955bdce71569ceab26a1ea71764cd Mon Sep 17 00:00:00 2001 From: vilmibm Date: Thu, 6 Feb 2020 13:03:20 -0600 Subject: [PATCH 07/51] use new goreleaser scoop config --- .goreleaser.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.goreleaser.yml b/.goreleaser.yml index 68691514a..1eb611313 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -87,5 +87,6 @@ scoop: name: vilmibm email: vilmibm@github.com homepage: https://github.com/cli/cli + skip_upload: auto description: GitHub CLI license: MIT From ff2d5fa6d07c533e2575cf06f95501fbb24ef254 Mon Sep 17 00:00:00 2001 From: Ahmed El Gabri Date: Fri, 7 Feb 2020 19:38:04 +0100 Subject: [PATCH 08/51] Check $GIT_EDITOR first --- pkg/surveyext/editor.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/surveyext/editor.go b/pkg/surveyext/editor.go index e49e9a6a7..003a96e5c 100644 --- a/pkg/surveyext/editor.go +++ b/pkg/surveyext/editor.go @@ -26,6 +26,9 @@ func init() { if runtime.GOOS == "windows" { editor = "notepad" } + if g := os.Getenv("GIT_EDITOR"); g != "" { + editor = g + } if v := os.Getenv("VISUAL"); v != "" { editor = v } From 2d10eb9b3210fe47acf526f7789378b5dfe7cfb4 Mon Sep 17 00:00:00 2001 From: Scott Penrose Date: Sun, 9 Feb 2020 00:16:47 -0500 Subject: [PATCH 09/51] Add .vscode and .DS_Store to .gitignore --- .gitignore | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.gitignore b/.gitignore index 76ddfcc68..b384e3d5c 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,9 @@ /site .github/**/node_modules /CHANGELOG.md + +# VS Code +.vscode + +# macOS +.DS_Store \ No newline at end of file From dd9b89b0c0d80ae76e03b4f7aa5542095261ae76 Mon Sep 17 00:00:00 2001 From: evelyn masso Date: Mon, 10 Feb 2020 10:33:35 -0800 Subject: [PATCH 10/51] first pr template draft --- .github/PULL_REQUEST_TEMPLATE/bug_fix.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 .github/PULL_REQUEST_TEMPLATE/bug_fix.md diff --git a/.github/PULL_REQUEST_TEMPLATE/bug_fix.md b/.github/PULL_REQUEST_TEMPLATE/bug_fix.md new file mode 100644 index 000000000..5659ce40b --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE/bug_fix.md @@ -0,0 +1,13 @@ + + +## Summary + +closes #[issue number] + +## Details + +- From e61b216b4fd12d96cdc509b3af63783419771eb0 Mon Sep 17 00:00:00 2001 From: Amanda Pinsker Date: Mon, 10 Feb 2020 16:15:02 -0800 Subject: [PATCH 11/51] Make approval statuses sentence case --- command/pr.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/command/pr.go b/command/pr.go index 38ee25c4e..dbef0f714 100644 --- a/command/pr.go +++ b/command/pr.go @@ -400,11 +400,11 @@ func printPrs(w io.Writer, totalCount int, prs ...api.PullRequest) { } if reviews.ChangesRequested { - fmt.Fprintf(w, " - %s", utils.Red("changes requested")) + fmt.Fprintf(w, " - %s", utils.Red("Changes requested")) } else if reviews.ReviewRequired { - fmt.Fprintf(w, " - %s", utils.Yellow("review required")) + fmt.Fprintf(w, " - %s", utils.Yellow("Review required")) } else if reviews.Approved { - fmt.Fprintf(w, " - %s", utils.Green("approved")) + fmt.Fprintf(w, " - %s", utils.Green("Approved")) } fmt.Fprint(w, "\n") From 1790d14c61b06a5e9ccffe1a533aad29ea2d9163 Mon Sep 17 00:00:00 2001 From: Amanda Pinsker Date: Mon, 10 Feb 2020 16:44:37 -0800 Subject: [PATCH 12/51] Update tests to reflect content change --- command/pr_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/command/pr_test.go b/command/pr_test.go index 3fbfb0b8e..cd22d5a85 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -111,9 +111,9 @@ func TestPRStatus_reviewsAndChecks(t *testing.T) { } expected := []string{ - "- Checks passing - changes requested", - "- Checks pending - approved", - "- 1/3 checks failing - review required", + "- Checks passing - Changes requested", + "- Checks pending - Approved", + "- 1/3 checks failing - Review required", } for _, line := range expected { From f0d8c651948f5418fd2ae93c45181ce585867d77 Mon Sep 17 00:00:00 2001 From: Borna Butkovic Date: Tue, 11 Feb 2020 01:25:09 +0100 Subject: [PATCH 13/51] pr view, status, list parent repo instead of fork --- command/pr.go | 28 ++++++++++++++++--- context/context.go | 68 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 4 deletions(-) diff --git a/command/pr.go b/command/pr.go index dbef0f714..f2d1ba3fe 100644 --- a/command/pr.go +++ b/command/pr.go @@ -29,8 +29,12 @@ func init() { prListCmd.Flags().StringP("base", "B", "", "Filter by base branch") prListCmd.Flags().StringSliceP("label", "l", nil, "Filter by label") prListCmd.Flags().StringP("assignee", "a", "", "Filter by assignee") + prListCmd.Flags().BoolP("self", "S", false, "Query current repository instead of forked parent") prViewCmd.Flags().BoolP("preview", "p", false, "Display preview of pull request content") + prViewCmd.Flags().BoolP("self", "S", false, "Query current repository instead of forked parent") + + prStatusCmd.Flags().BoolP("self", "S", false, "Query current repository instead of forked parent") } var prCmd = &cobra.Command{ @@ -70,7 +74,12 @@ func prStatus(cmd *cobra.Command, args []string) error { return err } - baseRepo, err := ctx.BaseRepo() + referSelf, _ := cmd.Flags().GetBool("self") + if referSelf == false { + ctx = context.ExpandOnline(ctx, apiClient) + } + + baseRepo, err := context.DetermineRepo(ctx, referSelf) if err != nil { return err } @@ -129,7 +138,12 @@ func prList(cmd *cobra.Command, args []string) error { return err } - baseRepo, err := ctx.BaseRepo() + referSelf, _ := cmd.Flags().GetBool("self") + if referSelf == false { + ctx = context.ExpandOnline(ctx, apiClient) + } + + baseRepo, err := context.DetermineRepo(ctx, referSelf) if err != nil { return err } @@ -240,12 +254,18 @@ func colorFuncForState(state string) func(string) string { func prView(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) - baseRepo, err := ctx.BaseRepo() + + apiClient, err := apiClientForContext(ctx) if err != nil { return err } - apiClient, err := apiClientForContext(ctx) + referSelf, _ := cmd.Flags().GetBool("self") + if referSelf == false { + ctx = context.ExpandOnline(ctx, apiClient) + } + + baseRepo, err := context.DetermineRepo(ctx, referSelf) if err != nil { return err } diff --git a/context/context.go b/context/context.go index 1ffd6bce6..dbf638854 100644 --- a/context/context.go +++ b/context/context.go @@ -1,8 +1,10 @@ package context import ( + "errors" "path" + "github.com/cli/cli/api" "github.com/cli/cli/git" "github.com/cli/cli/internal/ghrepo" "github.com/mitchellh/go-homedir" @@ -20,11 +22,45 @@ type Context interface { SetBaseRepo(string) } +type OnlineContext interface { + Context + ParentRepos() ([]ghrepo.Interface, error) +} + // New initializes a Context that reads from the filesystem func New() Context { return &fsContext{} } +func ExpandOnline(ctx Context, apiClient *api.Client) OnlineContext { + return &apiContext{ + Context: ctx, + apiClient: *apiClient, + } +} + +func DetermineRepo(ctx Context, self bool) (ghrepo.Interface, error) { + if self == true { + return ctx.BaseRepo() + } + + onlineCtx, isOnline := ctx.(OnlineContext) + if !isOnline { + return nil, errors.New("context not online") + } + + repos, err := onlineCtx.ParentRepos() + if err != nil { + return nil, err + } + + if len(repos) < 1 { + return ctx.BaseRepo() + } + + return repos[0], nil +} + // A Context implementation that queries the filesystem type fsContext struct { config *configEntry @@ -130,3 +166,35 @@ func (c *fsContext) BaseRepo() (ghrepo.Interface, error) { func (c *fsContext) SetBaseRepo(nwo string) { c.baseRepo = ghrepo.FromFullName(nwo) } + +type apiContext struct { + Context + apiClient api.Client +} + +func (c *apiContext) ParentRepos() ([]ghrepo.Interface, error) { + baseRepo, err := c.BaseRepo() + if err != nil { + return nil, err + } + + result, err := api.RepoNetwork(&c.apiClient, []ghrepo.Interface{baseRepo}) + if err != nil { + return nil, err + } + + if len(result.Repositories) < 1 { + return nil, errors.New("network request returned 0 repositories") + } + + var repos []ghrepo.Interface + + var repo api.Repository = *result.Repositories[0] + + for repo.IsFork() { + repo = *repo.Parent + repos = append(repos, repo) + } + + return repos, nil +} From 22be13d8d5dae7e395dd1f5bd5f183832101ac65 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 11 Feb 2020 11:16:16 -0600 Subject: [PATCH 14/51] move resolveRemotesToRepos to context --- command/pr_create.go | 98 +------------------------------------------- context/context.go | 96 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 97 deletions(-) diff --git a/command/pr_create.go b/command/pr_create.go index 5e2190cc0..5a844d324 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -1,10 +1,8 @@ package command import ( - "errors" "fmt" "net/url" - "sort" "time" "github.com/cli/cli/api" @@ -29,7 +27,7 @@ func prCreate(cmd *cobra.Command, _ []string) error { } baseRepoOverride, _ := cmd.Flags().GetString("repo") - repoContext, err := resolveRemotesToRepos(remotes, client, baseRepoOverride) + repoContext, err := context.ResolveRemotesToRepos(remotes, client, baseRepoOverride) if err != nil { return err } @@ -229,97 +227,3 @@ func init() { "The branch into which you want your code merged") prCreateCmd.Flags().BoolP("web", "w", false, "Open the web browser to create a pull request") } - -// cap the number of git remotes looked up, since the user might have an -// unusally large number of git remotes -const maxRemotesForLookup = 5 - -func resolveRemotesToRepos(remotes context.Remotes, client *api.Client, base string) (resolvedRemotes, error) { - sort.Stable(remotes) - lenRemotesForLookup := len(remotes) - if lenRemotesForLookup > maxRemotesForLookup { - lenRemotesForLookup = maxRemotesForLookup - } - - hasBaseOverride := base != "" - baseOverride := ghrepo.FromFullName(base) - foundBaseOverride := false - repos := []ghrepo.Interface{} - for _, r := range remotes[:lenRemotesForLookup] { - repos = append(repos, r) - if ghrepo.IsSame(r, baseOverride) { - foundBaseOverride = true - } - } - if hasBaseOverride && !foundBaseOverride { - // additionally, look up the explicitly specified base repo if it's not - // already covered by git remotes - repos = append(repos, baseOverride) - } - - result := resolvedRemotes{remotes: remotes} - if hasBaseOverride { - result.baseOverride = baseOverride - } - networkResult, err := api.RepoNetwork(client, repos) - if err != nil { - return result, err - } - result.network = networkResult - return result, nil -} - -type resolvedRemotes struct { - baseOverride ghrepo.Interface - remotes context.Remotes - network api.RepoNetworkResult -} - -// BaseRepo is the first found repository in the "upstream", "github", "origin" -// git remote order, resolved to the parent repo if the git remote points to a fork -func (r resolvedRemotes) BaseRepo() (*api.Repository, error) { - if r.baseOverride != nil { - for _, repo := range r.network.Repositories { - if repo != nil && ghrepo.IsSame(repo, r.baseOverride) { - return repo, nil - } - } - return nil, fmt.Errorf("failed looking up information about the '%s' repository", - ghrepo.FullName(r.baseOverride)) - } - - for _, repo := range r.network.Repositories { - if repo == nil { - continue - } - if repo.IsFork() { - return repo.Parent, nil - } - return repo, nil - } - - return nil, errors.New("not found") -} - -// HeadRepo is the first found repository that has push access -func (r resolvedRemotes) HeadRepo() (*api.Repository, error) { - for _, repo := range r.network.Repositories { - if repo != nil && repo.ViewerCanPush() { - return repo, nil - } - } - return nil, errors.New("none of the repositories have push access") -} - -// RemoteForRepo finds the git remote that points to a repository -func (r resolvedRemotes) RemoteForRepo(repo ghrepo.Interface) (*context.Remote, error) { - for i, remote := range r.remotes { - if ghrepo.IsSame(remote, repo) || - // additionally, look up the resolved repository name in case this - // git remote points to this repository via a redirect - (r.network.Repositories[i] != nil && ghrepo.IsSame(r.network.Repositories[i], repo)) { - return remote, nil - } - } - return nil, errors.New("not found") -} diff --git a/context/context.go b/context/context.go index dbf638854..40ea734ee 100644 --- a/context/context.go +++ b/context/context.go @@ -2,7 +2,9 @@ package context import ( "errors" + "fmt" "path" + "sort" "github.com/cli/cli/api" "github.com/cli/cli/git" @@ -22,6 +24,100 @@ type Context interface { SetBaseRepo(string) } +// cap the number of git remotes looked up, since the user might have an +// unusally large number of git remotes +const maxRemotesForLookup = 5 + +func ResolveRemotesToRepos(remotes Remotes, client *api.Client, base string) (ResolvedRemotes, error) { + sort.Stable(remotes) + lenRemotesForLookup := len(remotes) + if lenRemotesForLookup > maxRemotesForLookup { + lenRemotesForLookup = maxRemotesForLookup + } + + hasBaseOverride := base != "" + baseOverride := ghrepo.FromFullName(base) + foundBaseOverride := false + repos := []ghrepo.Interface{} + for _, r := range remotes[:lenRemotesForLookup] { + repos = append(repos, r) + if ghrepo.IsSame(r, baseOverride) { + foundBaseOverride = true + } + } + if hasBaseOverride && !foundBaseOverride { + // additionally, look up the explicitly specified base repo if it's not + // already covered by git remotes + repos = append(repos, baseOverride) + } + + result := ResolvedRemotes{remotes: remotes} + if hasBaseOverride { + result.baseOverride = baseOverride + } + networkResult, err := api.RepoNetwork(client, repos) + if err != nil { + return result, err + } + result.network = networkResult + return result, nil +} + +type ResolvedRemotes struct { + baseOverride ghrepo.Interface + remotes Remotes + network api.RepoNetworkResult +} + +// BaseRepo is the first found repository in the "upstream", "github", "origin" +// git remote order, resolved to the parent repo if the git remote points to a fork +func (r ResolvedRemotes) BaseRepo() (*api.Repository, error) { + if r.baseOverride != nil { + for _, repo := range r.network.Repositories { + if repo != nil && ghrepo.IsSame(repo, r.baseOverride) { + return repo, nil + } + } + return nil, fmt.Errorf("failed looking up information about the '%s' repository", + ghrepo.FullName(r.baseOverride)) + } + + for _, repo := range r.network.Repositories { + if repo == nil { + continue + } + if repo.IsFork() { + return repo.Parent, nil + } + return repo, nil + } + + return nil, errors.New("not found") +} + +// HeadRepo is the first found repository that has push access +func (r ResolvedRemotes) HeadRepo() (*api.Repository, error) { + for _, repo := range r.network.Repositories { + if repo != nil && repo.ViewerCanPush() { + return repo, nil + } + } + return nil, errors.New("none of the repositories have push access") +} + +// RemoteForRepo finds the git remote that points to a repository +func (r ResolvedRemotes) RemoteForRepo(repo ghrepo.Interface) (*Remote, error) { + for i, remote := range r.remotes { + if ghrepo.IsSame(remote, repo) || + // additionally, look up the resolved repository name in case this + // git remote points to this repository via a redirect + (r.network.Repositories[i] != nil && ghrepo.IsSame(r.network.Repositories[i], repo)) { + return remote, nil + } + } + return nil, errors.New("not found") +} + type OnlineContext interface { Context ParentRepos() ([]ghrepo.Interface, error) From f653dbb6b5d882268e1600da933a8976756821e5 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 11 Feb 2020 19:24:51 -0600 Subject: [PATCH 15/51] wrap and reuse the resolveToRemotes code from PR commands --- command/pr.go | 85 ++++++++++++++++++++++++++++++---------------- context/context.go | 66 ----------------------------------- 2 files changed, 55 insertions(+), 96 deletions(-) diff --git a/command/pr.go b/command/pr.go index f2d1ba3fe..c4b8eaa8b 100644 --- a/command/pr.go +++ b/command/pr.go @@ -67,6 +67,45 @@ branch is opened.`, RunE: prView, } +func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (*ghrepo.Interface, error) { + apiClient, err := apiClientForContext(ctx) + if err != nil { + return nil, err + } + + baseOverride, err := cmd.Flags().GetString("repo") + if err != nil { + return nil, err + } + + baseRepo, err := ctx.BaseRepo() + if err != nil { + return nil, err + } + + preferSelf, err := cmd.Flags().GetBool("self") + if err != nil { + return nil, err + } + + if preferSelf == false { + remotes, err := ctx.Remotes() + if err != nil { + return nil, err + } + + repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, baseOverride) + if err != nil { + return nil, err + } + baseRepo, err = repoContext.BaseRepo() + } + if err != nil { + return nil, err + } + return &baseRepo, nil +} + func prStatus(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) apiClient, err := apiClientForContext(ctx) @@ -74,15 +113,6 @@ func prStatus(cmd *cobra.Command, args []string) error { return err } - referSelf, _ := cmd.Flags().GetBool("self") - if referSelf == false { - ctx = context.ExpandOnline(ctx, apiClient) - } - - baseRepo, err := context.DetermineRepo(ctx, referSelf) - if err != nil { - return err - } currentPRNumber, currentPRHeadRef, err := prSelectorForCurrentBranch(ctx) if err != nil { return err @@ -92,7 +122,12 @@ func prStatus(cmd *cobra.Command, args []string) error { return err } - prPayload, err := api.PullRequests(apiClient, baseRepo, currentPRNumber, currentPRHeadRef, currentUser) + baseRepo, err := determineBaseRepo(cmd, ctx) + if err != nil { + return err + } + + prPayload, err := api.PullRequests(apiClient, *baseRepo, currentPRNumber, currentPRHeadRef, currentUser) if err != nil { return err } @@ -100,7 +135,7 @@ func prStatus(cmd *cobra.Command, args []string) error { out := colorableOut(cmd) fmt.Fprintln(out, "") - fmt.Fprintf(out, "Relevant pull requests in %s\n", ghrepo.FullName(baseRepo)) + fmt.Fprintf(out, "Relevant pull requests in %s\n", ghrepo.FullName(*baseRepo)) fmt.Fprintln(out, "") printHeader(out, "Current branch") @@ -138,17 +173,12 @@ func prList(cmd *cobra.Command, args []string) error { return err } - referSelf, _ := cmd.Flags().GetBool("self") - if referSelf == false { - ctx = context.ExpandOnline(ctx, apiClient) - } - - baseRepo, err := context.DetermineRepo(ctx, referSelf) + baseRepo, err := determineBaseRepo(cmd, ctx) if err != nil { return err } - fmt.Fprintf(colorableErr(cmd), "\nPull requests for %s\n\n", ghrepo.FullName(baseRepo)) + fmt.Fprintf(colorableErr(cmd), "\nPull requests for %s\n\n", ghrepo.FullName(*baseRepo)) limit, err := cmd.Flags().GetInt("limit") if err != nil { @@ -186,8 +216,8 @@ func prList(cmd *cobra.Command, args []string) error { } params := map[string]interface{}{ - "owner": baseRepo.RepoOwner(), - "repo": baseRepo.RepoName(), + "owner": (*baseRepo).RepoOwner(), + "repo": (*baseRepo).RepoName(), "state": graphqlState, } if len(labels) > 0 { @@ -260,12 +290,7 @@ func prView(cmd *cobra.Command, args []string) error { return err } - referSelf, _ := cmd.Flags().GetBool("self") - if referSelf == false { - ctx = context.ExpandOnline(ctx, apiClient) - } - - baseRepo, err := context.DetermineRepo(ctx, referSelf) + baseRepo, err := determineBaseRepo(cmd, ctx) if err != nil { return err } @@ -278,7 +303,7 @@ func prView(cmd *cobra.Command, args []string) error { var openURL string var pr *api.PullRequest if len(args) > 0 { - pr, err = prFromArg(apiClient, baseRepo, args[0]) + pr, err = prFromArg(apiClient, *baseRepo, args[0]) if err != nil { return err } @@ -290,15 +315,15 @@ func prView(cmd *cobra.Command, args []string) error { } if prNumber > 0 { - openURL = fmt.Sprintf("https://github.com/%s/pull/%d", ghrepo.FullName(baseRepo), prNumber) + openURL = fmt.Sprintf("https://github.com/%s/pull/%d", ghrepo.FullName(*baseRepo), prNumber) if preview { - pr, err = api.PullRequestByNumber(apiClient, baseRepo, prNumber) + pr, err = api.PullRequestByNumber(apiClient, *baseRepo, prNumber) if err != nil { return err } } } else { - pr, err = api.PullRequestForBranch(apiClient, baseRepo, branchWithOwner) + pr, err = api.PullRequestForBranch(apiClient, *baseRepo, branchWithOwner) if err != nil { return err } diff --git a/context/context.go b/context/context.go index 40ea734ee..495c6aa71 100644 --- a/context/context.go +++ b/context/context.go @@ -118,45 +118,11 @@ func (r ResolvedRemotes) RemoteForRepo(repo ghrepo.Interface) (*Remote, error) { return nil, errors.New("not found") } -type OnlineContext interface { - Context - ParentRepos() ([]ghrepo.Interface, error) -} - // New initializes a Context that reads from the filesystem func New() Context { return &fsContext{} } -func ExpandOnline(ctx Context, apiClient *api.Client) OnlineContext { - return &apiContext{ - Context: ctx, - apiClient: *apiClient, - } -} - -func DetermineRepo(ctx Context, self bool) (ghrepo.Interface, error) { - if self == true { - return ctx.BaseRepo() - } - - onlineCtx, isOnline := ctx.(OnlineContext) - if !isOnline { - return nil, errors.New("context not online") - } - - repos, err := onlineCtx.ParentRepos() - if err != nil { - return nil, err - } - - if len(repos) < 1 { - return ctx.BaseRepo() - } - - return repos[0], nil -} - // A Context implementation that queries the filesystem type fsContext struct { config *configEntry @@ -262,35 +228,3 @@ func (c *fsContext) BaseRepo() (ghrepo.Interface, error) { func (c *fsContext) SetBaseRepo(nwo string) { c.baseRepo = ghrepo.FromFullName(nwo) } - -type apiContext struct { - Context - apiClient api.Client -} - -func (c *apiContext) ParentRepos() ([]ghrepo.Interface, error) { - baseRepo, err := c.BaseRepo() - if err != nil { - return nil, err - } - - result, err := api.RepoNetwork(&c.apiClient, []ghrepo.Interface{baseRepo}) - if err != nil { - return nil, err - } - - if len(result.Repositories) < 1 { - return nil, errors.New("network request returned 0 repositories") - } - - var repos []ghrepo.Interface - - var repo api.Repository = *result.Repositories[0] - - for repo.IsFork() { - repo = *repo.Parent - repos = append(repos, repo) - } - - return repos, nil -} From ad170118e4965375c7cef0ac99a734bddda0fdad Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 11 Feb 2020 19:43:02 -0600 Subject: [PATCH 16/51] use resolve remotes code in issue commands --- command/issue.go | 41 ++++++++++++++++++++++++----------------- command/pr.go | 5 +++++ 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/command/issue.go b/command/issue.go index b9987d0e5..721dc0f98 100644 --- a/command/issue.go +++ b/command/issue.go @@ -30,14 +30,19 @@ func init() { issueCreateCmd.Flags().StringP("body", "b", "", "Supply a body. Will prompt for one otherwise.") issueCreateCmd.Flags().BoolP("web", "w", false, "Open the browser to create an issue") + issueCreateCmd.Flags().BoolP("self", "S", false, "Query current repository instead of forked parent") issueCmd.AddCommand(issueListCmd) issueListCmd.Flags().StringP("assignee", "a", "", "Filter by assignee") issueListCmd.Flags().StringSliceP("label", "l", nil, "Filter by label") issueListCmd.Flags().StringP("state", "s", "", "Filter by state: {open|closed|all}") issueListCmd.Flags().IntP("limit", "L", 30, "Maximum number of issues to fetch") + issueListCmd.Flags().BoolP("self", "S", false, "Query current repository instead of forked parent") issueViewCmd.Flags().BoolP("preview", "p", false, "Display preview of issue content") + issueViewCmd.Flags().BoolP("self", "S", false, "Query current repository instead of forked parent") + + issueStatusCmd.Flags().BoolP("self", "S", false, "Query current repository instead of forked parent") } var issueCmd = &cobra.Command{ @@ -83,7 +88,7 @@ func issueList(cmd *cobra.Command, args []string) error { return err } - baseRepo, err := ctx.BaseRepo() + baseRepo, err := determineBaseRepo(cmd, ctx) if err != nil { return err } @@ -108,9 +113,9 @@ func issueList(cmd *cobra.Command, args []string) error { return err } - fmt.Fprintf(colorableErr(cmd), "\nIssues for %s\n\n", ghrepo.FullName(baseRepo)) + fmt.Fprintf(colorableErr(cmd), "\nIssues for %s\n\n", ghrepo.FullName(*baseRepo)) - issues, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit) + issues, err := api.IssueList(apiClient, *baseRepo, state, labels, assignee, limit) if err != nil { return err } @@ -158,7 +163,7 @@ func issueStatus(cmd *cobra.Command, args []string) error { return err } - baseRepo, err := ctx.BaseRepo() + baseRepo, err := determineBaseRepo(cmd, ctx) if err != nil { return err } @@ -168,7 +173,7 @@ func issueStatus(cmd *cobra.Command, args []string) error { return err } - issuePayload, err := api.IssueStatus(apiClient, baseRepo, currentUser) + issuePayload, err := api.IssueStatus(apiClient, *baseRepo, currentUser) if err != nil { return err } @@ -176,7 +181,7 @@ func issueStatus(cmd *cobra.Command, args []string) error { out := colorableOut(cmd) fmt.Fprintln(out, "") - fmt.Fprintf(out, "Relevant issues in %s\n", ghrepo.FullName(baseRepo)) + fmt.Fprintf(out, "Relevant issues in %s\n", ghrepo.FullName(*baseRepo)) fmt.Fprintln(out, "") printHeader(out, "Issues assigned to you") @@ -210,16 +215,17 @@ func issueStatus(cmd *cobra.Command, args []string) error { func issueView(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) - baseRepo, err := ctx.BaseRepo() - if err != nil { - return err - } apiClient, err := apiClientForContext(ctx) if err != nil { return err } - issue, err := issueFromArg(apiClient, baseRepo, args[0]) + baseRepo, err := determineBaseRepo(cmd, ctx) + if err != nil { + return err + } + + issue, err := issueFromArg(apiClient, *baseRepo, args[0]) if err != nil { return err } @@ -278,12 +284,13 @@ func issueFromArg(apiClient *api.Client, baseRepo ghrepo.Interface, arg string) func issueCreate(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) - baseRepo, err := ctx.BaseRepo() + // NB no auto forking like over in pr create + baseRepo, err := determineBaseRepo(cmd, ctx) if err != nil { return err } - fmt.Fprintf(colorableErr(cmd), "\nCreating issue in %s\n\n", ghrepo.FullName(baseRepo)) + fmt.Fprintf(colorableErr(cmd), "\nCreating issue in %s\n\n", ghrepo.FullName(*baseRepo)) var templateFiles []string if rootDir, err := git.ToplevelDir(); err == nil { @@ -293,7 +300,7 @@ func issueCreate(cmd *cobra.Command, args []string) error { if isWeb, err := cmd.Flags().GetBool("web"); err == nil && isWeb { // TODO: move URL generation into GitHubRepository - openURL := fmt.Sprintf("https://github.com/%s/issues/new", ghrepo.FullName(baseRepo)) + openURL := fmt.Sprintf("https://github.com/%s/issues/new", ghrepo.FullName(*baseRepo)) if len(templateFiles) > 1 { openURL += "/choose" } @@ -306,12 +313,12 @@ func issueCreate(cmd *cobra.Command, args []string) error { return err } - repo, err := api.GitHubRepo(apiClient, baseRepo) + repo, err := api.GitHubRepo(apiClient, *baseRepo) if err != nil { return err } if !repo.HasIssuesEnabled { - return fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(baseRepo)) + return fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(*baseRepo)) } action := SubmitAction @@ -352,7 +359,7 @@ func issueCreate(cmd *cobra.Command, args []string) error { if action == PreviewAction { openURL := fmt.Sprintf( "https://github.com/%s/issues/new/?title=%s&body=%s", - ghrepo.FullName(baseRepo), + ghrepo.FullName(*baseRepo), url.QueryEscape(title), url.QueryEscape(body), ) diff --git a/command/pr.go b/command/pr.go index c4b8eaa8b..777e29ed6 100644 --- a/command/pr.go +++ b/command/pr.go @@ -94,6 +94,11 @@ func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (*ghrepo.Interfa return nil, err } + // TODO given remotes: + // [upstream] cli/cli + // [origin] vilmibm/cli + // we're picking the wrong thing. i think preferSelf needs to be threaded through to + // ResolveRemotesToRepos in the same way as baseOverride. repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, baseOverride) if err != nil { return nil, err From c93c5f766810ca5e316bc537e20187c666482b9d Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 11 Feb 2020 19:44:49 -0600 Subject: [PATCH 17/51] move determineBaseRepo to root --- command/pr.go | 44 -------------------------------------------- command/root.go | 45 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 44 deletions(-) diff --git a/command/pr.go b/command/pr.go index 777e29ed6..4bd3059eb 100644 --- a/command/pr.go +++ b/command/pr.go @@ -67,50 +67,6 @@ branch is opened.`, RunE: prView, } -func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (*ghrepo.Interface, error) { - apiClient, err := apiClientForContext(ctx) - if err != nil { - return nil, err - } - - baseOverride, err := cmd.Flags().GetString("repo") - if err != nil { - return nil, err - } - - baseRepo, err := ctx.BaseRepo() - if err != nil { - return nil, err - } - - preferSelf, err := cmd.Flags().GetBool("self") - if err != nil { - return nil, err - } - - if preferSelf == false { - remotes, err := ctx.Remotes() - if err != nil { - return nil, err - } - - // TODO given remotes: - // [upstream] cli/cli - // [origin] vilmibm/cli - // we're picking the wrong thing. i think preferSelf needs to be threaded through to - // ResolveRemotesToRepos in the same way as baseOverride. - repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, baseOverride) - if err != nil { - return nil, err - } - baseRepo, err = repoContext.BaseRepo() - } - if err != nil { - return nil, err - } - return &baseRepo, nil -} - func prStatus(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) apiClient, err := apiClientForContext(ctx) diff --git a/command/root.go b/command/root.go index 1329569e4..d3078dee2 100644 --- a/command/root.go +++ b/command/root.go @@ -9,6 +9,7 @@ import ( "github.com/cli/cli/api" "github.com/cli/cli/context" + "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/utils" "github.com/spf13/cobra" @@ -151,3 +152,47 @@ func changelogURL(version string) string { url := fmt.Sprintf("%s/releases/tag/v%s", path, strings.TrimPrefix(version, "v")) return url } + +func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (*ghrepo.Interface, error) { + apiClient, err := apiClientForContext(ctx) + if err != nil { + return nil, err + } + + baseOverride, err := cmd.Flags().GetString("repo") + if err != nil { + return nil, err + } + + baseRepo, err := ctx.BaseRepo() + if err != nil { + return nil, err + } + + preferSelf, err := cmd.Flags().GetBool("self") + if err != nil { + return nil, err + } + + if preferSelf == false { + remotes, err := ctx.Remotes() + if err != nil { + return nil, err + } + + // TODO given remotes: + // [upstream] cli/cli + // [origin] vilmibm/cli + // we're picking the wrong thing. i think preferSelf needs to be threaded through to + // ResolveRemotesToRepos in the same way as baseOverride. + repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, baseOverride) + if err != nil { + return nil, err + } + baseRepo, err = repoContext.BaseRepo() + } + if err != nil { + return nil, err + } + return &baseRepo, nil +} From 135c63aa363a873719e6f132f2692a0cd07cc48a Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 11 Feb 2020 20:46:46 -0600 Subject: [PATCH 18/51] remove --self for now --- command/issue.go | 5 ----- command/pr.go | 4 ---- command/root.go | 29 +++++++---------------------- 3 files changed, 7 insertions(+), 31 deletions(-) diff --git a/command/issue.go b/command/issue.go index 721dc0f98..0d11f5ad4 100644 --- a/command/issue.go +++ b/command/issue.go @@ -30,19 +30,14 @@ func init() { issueCreateCmd.Flags().StringP("body", "b", "", "Supply a body. Will prompt for one otherwise.") issueCreateCmd.Flags().BoolP("web", "w", false, "Open the browser to create an issue") - issueCreateCmd.Flags().BoolP("self", "S", false, "Query current repository instead of forked parent") issueCmd.AddCommand(issueListCmd) issueListCmd.Flags().StringP("assignee", "a", "", "Filter by assignee") issueListCmd.Flags().StringSliceP("label", "l", nil, "Filter by label") issueListCmd.Flags().StringP("state", "s", "", "Filter by state: {open|closed|all}") issueListCmd.Flags().IntP("limit", "L", 30, "Maximum number of issues to fetch") - issueListCmd.Flags().BoolP("self", "S", false, "Query current repository instead of forked parent") issueViewCmd.Flags().BoolP("preview", "p", false, "Display preview of issue content") - issueViewCmd.Flags().BoolP("self", "S", false, "Query current repository instead of forked parent") - - issueStatusCmd.Flags().BoolP("self", "S", false, "Query current repository instead of forked parent") } var issueCmd = &cobra.Command{ diff --git a/command/pr.go b/command/pr.go index 4bd3059eb..039b8c37a 100644 --- a/command/pr.go +++ b/command/pr.go @@ -29,12 +29,8 @@ func init() { prListCmd.Flags().StringP("base", "B", "", "Filter by base branch") prListCmd.Flags().StringSliceP("label", "l", nil, "Filter by label") prListCmd.Flags().StringP("assignee", "a", "", "Filter by assignee") - prListCmd.Flags().BoolP("self", "S", false, "Query current repository instead of forked parent") prViewCmd.Flags().BoolP("preview", "p", false, "Display preview of pull request content") - prViewCmd.Flags().BoolP("self", "S", false, "Query current repository instead of forked parent") - - prStatusCmd.Flags().BoolP("self", "S", false, "Query current repository instead of forked parent") } var prCmd = &cobra.Command{ diff --git a/command/root.go b/command/root.go index d3078dee2..7b9b62ce1 100644 --- a/command/root.go +++ b/command/root.go @@ -164,35 +164,20 @@ func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (*ghrepo.Interfa return nil, err } - baseRepo, err := ctx.BaseRepo() + remotes, err := ctx.Remotes() if err != nil { return nil, err } - preferSelf, err := cmd.Flags().GetBool("self") + repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, baseOverride) + if err != nil { + return nil, err + } + var baseRepo ghrepo.Interface + baseRepo, err = repoContext.BaseRepo() if err != nil { return nil, err } - if preferSelf == false { - remotes, err := ctx.Remotes() - if err != nil { - return nil, err - } - - // TODO given remotes: - // [upstream] cli/cli - // [origin] vilmibm/cli - // we're picking the wrong thing. i think preferSelf needs to be threaded through to - // ResolveRemotesToRepos in the same way as baseOverride. - repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, baseOverride) - if err != nil { - return nil, err - } - baseRepo, err = repoContext.BaseRepo() - } - if err != nil { - return nil, err - } return &baseRepo, nil } From c8a4ac66d848cf732be2d7b18c17b1ec3691f9e0 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 11 Feb 2020 21:56:20 -0600 Subject: [PATCH 19/51] start on fixing tests --- command/issue_test.go | 110 ++++++++++++++++++++++++++++++++++++-- command/pr_create_test.go | 110 -------------------------------------- context/context.go | 28 +++++----- context/remote_test.go | 110 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 229 insertions(+), 129 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index 98ba6119e..334554789 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -9,13 +9,33 @@ import ( "regexp" "testing" + "github.com/cli/cli/context" "github.com/cli/cli/utils" ) func TestIssueStatus(t *testing.T) { - initBlankContext("OWNER/REPO", "master") + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) jsonFile, _ := os.Open("../test/fixtures/issueStatus.json") defer jsonFile.Close() http.StubResponse(200, jsonFile) @@ -41,9 +61,29 @@ func TestIssueStatus(t *testing.T) { } func TestIssueStatus_blankSlate(t *testing.T) { - initBlankContext("OWNER/REPO", "master") + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "hasIssuesEnabled": true, @@ -77,9 +117,29 @@ Issues opened by you } func TestIssueStatus_disabledIssues(t *testing.T) { - initBlankContext("OWNER/REPO", "master") + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "hasIssuesEnabled": false @@ -93,9 +153,29 @@ func TestIssueStatus_disabledIssues(t *testing.T) { } func TestIssueList(t *testing.T) { - initBlankContext("OWNER/REPO", "master") + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + jsonFile, _ := os.Open("../test/fixtures/issueList.json") defer jsonFile.Close() http.StubResponse(200, jsonFile) @@ -120,9 +200,29 @@ func TestIssueList(t *testing.T) { } func TestIssueList_withFlags(t *testing.T) { - initBlankContext("OWNER/REPO", "master") + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "hasIssuesEnabled": true, diff --git a/command/pr_create_test.go b/command/pr_create_test.go index e6e8c370f..4a901ed3a 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -10,10 +10,8 @@ import ( "strings" "testing" - "github.com/cli/cli/api" "github.com/cli/cli/context" "github.com/cli/cli/git" - "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/test" "github.com/cli/cli/utils" ) @@ -190,111 +188,3 @@ Creating pull request for feature into master in OWNER/REPO `) } - -func Test_resolvedRemotes_clonedFork(t *testing.T) { - resolved := resolvedRemotes{ - baseOverride: nil, - remotes: context.Remotes{ - &context.Remote{ - Remote: &git.Remote{Name: "origin"}, - Owner: "OWNER", - Repo: "REPO", - }, - }, - network: api.RepoNetworkResult{ - Repositories: []*api.Repository{ - &api.Repository{ - Name: "REPO", - Owner: api.RepositoryOwner{Login: "OWNER"}, - ViewerPermission: "ADMIN", - Parent: &api.Repository{ - Name: "REPO", - Owner: api.RepositoryOwner{Login: "PARENTOWNER"}, - ViewerPermission: "READ", - }, - }, - }, - }, - } - - baseRepo, err := resolved.BaseRepo() - if err != nil { - t.Fatalf("got %v", err) - } - eq(t, ghrepo.FullName(baseRepo), "PARENTOWNER/REPO") - baseRemote, err := resolved.RemoteForRepo(baseRepo) - if baseRemote != nil || err == nil { - t.Error("did not expect any remote for base") - } - - headRepo, err := resolved.HeadRepo() - if err != nil { - t.Fatalf("got %v", err) - } - eq(t, ghrepo.FullName(headRepo), "OWNER/REPO") - headRemote, err := resolved.RemoteForRepo(headRepo) - if err != nil { - t.Fatalf("got %v", err) - } - if headRemote.Name != "origin" { - t.Errorf("got remote %q", headRemote.Name) - } -} - -func Test_resolvedRemotes_triangularSetup(t *testing.T) { - resolved := resolvedRemotes{ - baseOverride: nil, - remotes: context.Remotes{ - &context.Remote{ - Remote: &git.Remote{Name: "origin"}, - Owner: "OWNER", - Repo: "REPO", - }, - &context.Remote{ - Remote: &git.Remote{Name: "fork"}, - Owner: "MYSELF", - Repo: "REPO", - }, - }, - network: api.RepoNetworkResult{ - Repositories: []*api.Repository{ - &api.Repository{ - Name: "NEWNAME", - Owner: api.RepositoryOwner{Login: "NEWOWNER"}, - ViewerPermission: "READ", - }, - &api.Repository{ - Name: "REPO", - Owner: api.RepositoryOwner{Login: "MYSELF"}, - ViewerPermission: "ADMIN", - }, - }, - }, - } - - baseRepo, err := resolved.BaseRepo() - if err != nil { - t.Fatalf("got %v", err) - } - eq(t, ghrepo.FullName(baseRepo), "NEWOWNER/NEWNAME") - baseRemote, err := resolved.RemoteForRepo(baseRepo) - if err != nil { - t.Fatalf("got %v", err) - } - if baseRemote.Name != "origin" { - t.Errorf("got remote %q", baseRemote.Name) - } - - headRepo, err := resolved.HeadRepo() - if err != nil { - t.Fatalf("got %v", err) - } - eq(t, ghrepo.FullName(headRepo), "MYSELF/REPO") - headRemote, err := resolved.RemoteForRepo(headRepo) - if err != nil { - t.Fatalf("got %v", err) - } - if headRemote.Name != "fork" { - t.Errorf("got remote %q", headRemote.Name) - } -} diff --git a/context/context.go b/context/context.go index 495c6aa71..fed589a8d 100644 --- a/context/context.go +++ b/context/context.go @@ -51,38 +51,38 @@ func ResolveRemotesToRepos(remotes Remotes, client *api.Client, base string) (Re repos = append(repos, baseOverride) } - result := ResolvedRemotes{remotes: remotes} + result := ResolvedRemotes{Remotes: remotes} if hasBaseOverride { - result.baseOverride = baseOverride + result.BaseOverride = baseOverride } networkResult, err := api.RepoNetwork(client, repos) if err != nil { return result, err } - result.network = networkResult + result.Network = networkResult return result, nil } type ResolvedRemotes struct { - baseOverride ghrepo.Interface - remotes Remotes - network api.RepoNetworkResult + BaseOverride ghrepo.Interface + Remotes Remotes + Network api.RepoNetworkResult } // BaseRepo is the first found repository in the "upstream", "github", "origin" // git remote order, resolved to the parent repo if the git remote points to a fork func (r ResolvedRemotes) BaseRepo() (*api.Repository, error) { - if r.baseOverride != nil { - for _, repo := range r.network.Repositories { - if repo != nil && ghrepo.IsSame(repo, r.baseOverride) { + if r.BaseOverride != nil { + for _, repo := range r.Network.Repositories { + if repo != nil && ghrepo.IsSame(repo, r.BaseOverride) { return repo, nil } } return nil, fmt.Errorf("failed looking up information about the '%s' repository", - ghrepo.FullName(r.baseOverride)) + ghrepo.FullName(r.BaseOverride)) } - for _, repo := range r.network.Repositories { + for _, repo := range r.Network.Repositories { if repo == nil { continue } @@ -97,7 +97,7 @@ func (r ResolvedRemotes) BaseRepo() (*api.Repository, error) { // HeadRepo is the first found repository that has push access func (r ResolvedRemotes) HeadRepo() (*api.Repository, error) { - for _, repo := range r.network.Repositories { + for _, repo := range r.Network.Repositories { if repo != nil && repo.ViewerCanPush() { return repo, nil } @@ -107,11 +107,11 @@ func (r ResolvedRemotes) HeadRepo() (*api.Repository, error) { // RemoteForRepo finds the git remote that points to a repository func (r ResolvedRemotes) RemoteForRepo(repo ghrepo.Interface) (*Remote, error) { - for i, remote := range r.remotes { + for i, remote := range r.Remotes { if ghrepo.IsSame(remote, repo) || // additionally, look up the resolved repository name in case this // git remote points to this repository via a redirect - (r.network.Repositories[i] != nil && ghrepo.IsSame(r.network.Repositories[i], repo)) { + (r.Network.Repositories[i] != nil && ghrepo.IsSame(r.Network.Repositories[i], repo)) { return remote, nil } } diff --git a/context/remote_test.go b/context/remote_test.go index 727ddf078..04ccbc56c 100644 --- a/context/remote_test.go +++ b/context/remote_test.go @@ -5,7 +5,9 @@ import ( "net/url" "testing" + "github.com/cli/cli/api" "github.com/cli/cli/git" + "github.com/cli/cli/internal/ghrepo" ) func Test_Remotes_FindByName(t *testing.T) { @@ -57,3 +59,111 @@ func Test_translateRemotes(t *testing.T) { t.Errorf("got %q", result[0].RepoName()) } } + +func Test_resolvedRemotes_triangularSetup(t *testing.T) { + resolved := ResolvedRemotes{ + BaseOverride: nil, + Remotes: Remotes{ + &Remote{ + Remote: &git.Remote{Name: "origin"}, + Owner: "OWNER", + Repo: "REPO", + }, + &Remote{ + Remote: &git.Remote{Name: "fork"}, + Owner: "MYSELF", + Repo: "REPO", + }, + }, + Network: api.RepoNetworkResult{ + Repositories: []*api.Repository{ + &api.Repository{ + Name: "NEWNAME", + Owner: api.RepositoryOwner{Login: "NEWOWNER"}, + ViewerPermission: "READ", + }, + &api.Repository{ + Name: "REPO", + Owner: api.RepositoryOwner{Login: "MYSELF"}, + ViewerPermission: "ADMIN", + }, + }, + }, + } + + baseRepo, err := resolved.BaseRepo() + if err != nil { + t.Fatalf("got %v", err) + } + eq(t, ghrepo.FullName(baseRepo), "NEWOWNER/NEWNAME") + baseRemote, err := resolved.RemoteForRepo(baseRepo) + if err != nil { + t.Fatalf("got %v", err) + } + if baseRemote.Name != "origin" { + t.Errorf("got remote %q", baseRemote.Name) + } + + headRepo, err := resolved.HeadRepo() + if err != nil { + t.Fatalf("got %v", err) + } + eq(t, ghrepo.FullName(headRepo), "MYSELF/REPO") + headRemote, err := resolved.RemoteForRepo(headRepo) + if err != nil { + t.Fatalf("got %v", err) + } + if headRemote.Name != "fork" { + t.Errorf("got remote %q", headRemote.Name) + } +} + +func Test_resolvedRemotes_clonedFork(t *testing.T) { + resolved := ResolvedRemotes{ + BaseOverride: nil, + Remotes: Remotes{ + &Remote{ + Remote: &git.Remote{Name: "origin"}, + Owner: "OWNER", + Repo: "REPO", + }, + }, + Network: api.RepoNetworkResult{ + Repositories: []*api.Repository{ + &api.Repository{ + Name: "REPO", + Owner: api.RepositoryOwner{Login: "OWNER"}, + ViewerPermission: "ADMIN", + Parent: &api.Repository{ + Name: "REPO", + Owner: api.RepositoryOwner{Login: "PARENTOWNER"}, + ViewerPermission: "READ", + }, + }, + }, + }, + } + + baseRepo, err := resolved.BaseRepo() + if err != nil { + t.Fatalf("got %v", err) + } + eq(t, ghrepo.FullName(baseRepo), "PARENTOWNER/REPO") + baseRemote, err := resolved.RemoteForRepo(baseRepo) + if baseRemote != nil || err == nil { + t.Error("did not expect any remote for base") + } + + headRepo, err := resolved.HeadRepo() + if err != nil { + t.Fatalf("got %v", err) + } + eq(t, ghrepo.FullName(headRepo), "OWNER/REPO") + headRemote, err := resolved.RemoteForRepo(headRepo) + if err != nil { + t.Fatalf("got %v", err) + } + if headRemote.Name != "origin" { + t.Errorf("got remote %q", headRemote.Name) + } +} From a998a650cdd88a75122db4832996507853283196 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 12 Feb 2020 08:15:10 -0600 Subject: [PATCH 20/51] still WIP, need to pause and refactor --- command/issue_test.go | 90 +++++++++++- command/pr_test.go | 309 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 380 insertions(+), 19 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index 334554789..afc0d0c2e 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -258,9 +258,29 @@ No issues match your search } func TestIssueList_nullAssigneeLabels(t *testing.T) { - initBlankContext("OWNER/REPO", "master") + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "hasIssuesEnabled": true, @@ -456,9 +476,29 @@ func TestIssueView_urlArg(t *testing.T) { } func TestIssueCreate(t *testing.T) { - initBlankContext("OWNER/REPO", "master") + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "id": "REPOID", @@ -496,9 +536,29 @@ func TestIssueCreate(t *testing.T) { } func TestIssueCreate_disabledIssues(t *testing.T) { - initBlankContext("OWNER/REPO", "master") + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "id": "REPOID", @@ -513,8 +573,28 @@ func TestIssueCreate_disabledIssues(t *testing.T) { } func TestIssueCreate_web(t *testing.T) { - initBlankContext("OWNER/REPO", "master") - initFakeHTTP() + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) var seenCmd *exec.Cmd restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { diff --git a/command/pr_test.go b/command/pr_test.go index cd22d5a85..d194cf726 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -11,6 +11,7 @@ import ( "strings" "testing" + "github.com/cli/cli/context" "github.com/cli/cli/utils" "github.com/google/shlex" "github.com/spf13/cobra" @@ -71,9 +72,29 @@ func RunCommand(cmd *cobra.Command, args string) (*cmdOut, error) { } func TestPRStatus(t *testing.T) { - initBlankContext("OWNER/REPO", "blueberries") + ctx := context.NewBlank() + ctx.SetBranch("blueberries") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + jsonFile, _ := os.Open("../test/fixtures/prStatus.json") defer jsonFile.Close() http.StubResponse(200, jsonFile) @@ -98,9 +119,29 @@ func TestPRStatus(t *testing.T) { } func TestPRStatus_reviewsAndChecks(t *testing.T) { - initBlankContext("OWNER/REPO", "blueberries") + ctx := context.NewBlank() + ctx.SetBranch("blueberries") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + jsonFile, _ := os.Open("../test/fixtures/prStatusChecks.json") defer jsonFile.Close() http.StubResponse(200, jsonFile) @@ -124,9 +165,29 @@ func TestPRStatus_reviewsAndChecks(t *testing.T) { } func TestPRStatus_blankSlate(t *testing.T) { - initBlankContext("OWNER/REPO", "blueberries") + ctx := context.NewBlank() + ctx.SetBranch("blueberries") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(` { "data": {} } `)) @@ -155,9 +216,29 @@ Requesting a code review from you } func TestPRList(t *testing.T) { - initBlankContext("OWNER/REPO", "master") + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + jsonFile, _ := os.Open("../test/fixtures/prList.json") defer jsonFile.Close() http.StubResponse(200, jsonFile) @@ -174,9 +255,29 @@ func TestPRList(t *testing.T) { } func TestPRList_filtering(t *testing.T) { - initBlankContext("OWNER/REPO", "master") + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + respBody := bytes.NewBufferString(`{ "data": {} }`) http.StubResponse(200, respBody) @@ -206,9 +307,29 @@ No pull requests match your search } func TestPRList_filteringAssignee(t *testing.T) { - initBlankContext("OWNER/REPO", "master") + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + respBody := bytes.NewBufferString(`{ "data": {} }`) http.StubResponse(200, respBody) @@ -242,9 +363,29 @@ func TestPRList_filteringAssigneeLabels(t *testing.T) { } func TestPRView_preview(t *testing.T) { - initBlankContext("OWNER/REPO", "master") + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + jsonFile, _ := os.Open("../test/fixtures/prViewPreview.json") defer jsonFile.Close() http.StubResponse(200, jsonFile) @@ -271,9 +412,29 @@ func TestPRView_preview(t *testing.T) { } func TestPRView_previewCurrentBranch(t *testing.T) { - initBlankContext("OWNER/REPO", "blueberries") + ctx := context.NewBlank() + ctx.SetBranch("blueberries") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + jsonFile, _ := os.Open("../test/fixtures/prView.json") defer jsonFile.Close() http.StubResponse(200, jsonFile) @@ -305,9 +466,29 @@ func TestPRView_previewCurrentBranch(t *testing.T) { } func TestPRView_currentBranch(t *testing.T) { - initBlankContext("OWNER/REPO", "blueberries") + ctx := context.NewBlank() + ctx.SetBranch("blueberries") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + jsonFile, _ := os.Open("../test/fixtures/prView.json") defer jsonFile.Close() http.StubResponse(200, jsonFile) @@ -342,9 +523,29 @@ func TestPRView_currentBranch(t *testing.T) { } func TestPRView_noResultsForBranch(t *testing.T) { - initBlankContext("OWNER/REPO", "blueberries") + ctx := context.NewBlank() + ctx.SetBranch("blueberries") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + jsonFile, _ := os.Open("../test/fixtures/prView_NoActiveBranch.json") defer jsonFile.Close() http.StubResponse(200, jsonFile) @@ -372,9 +573,29 @@ func TestPRView_noResultsForBranch(t *testing.T) { } func TestPRView_numberArg(t *testing.T) { - initBlankContext("OWNER/REPO", "master") + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { "url": "https://github.com/OWNER/REPO/pull/23" @@ -403,9 +624,29 @@ func TestPRView_numberArg(t *testing.T) { } func TestPRView_urlArg(t *testing.T) { - initBlankContext("OWNER/REPO", "master") + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { "url": "https://github.com/OWNER/REPO/pull/23" @@ -434,9 +675,29 @@ func TestPRView_urlArg(t *testing.T) { } func TestPRView_branchArg(t *testing.T) { - initBlankContext("OWNER/REPO", "master") + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequests": { "nodes": [ { "headRefName": "blueberries", @@ -467,9 +728,29 @@ func TestPRView_branchArg(t *testing.T) { } func TestPRView_branchWithOwnerArg(t *testing.T) { - initBlankContext("OWNER/REPO", "master") + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequests": { "nodes": [ { "headRefName": "blueberries", From dd57b891d350ba977a6b44c1aca03955ce16bb04 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 12 Feb 2020 08:48:04 -0600 Subject: [PATCH 21/51] add helper for stubbing repo resolve response --- api/fake_http.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/api/fake_http.go b/api/fake_http.go index 96e38aab3..32b7d56eb 100644 --- a/api/fake_http.go +++ b/api/fake_http.go @@ -1,6 +1,7 @@ package api import ( + "bytes" "fmt" "io" "io/ioutil" @@ -35,3 +36,23 @@ func (f *FakeHTTP) RoundTrip(req *http.Request) (*http.Response, error) { f.Requests = append(f.Requests, req) return resp, nil } + +func (f *FakeHTTP) StubRepoResponse(owner, repo string) { + body := bytes.NewBufferString(fmt.Sprintf(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "%s", + "owner": {"login": "%s"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `, repo, owner)) + resp := &http.Response{ + StatusCode: 200, + Body: ioutil.NopCloser(body), + } + f.responseStubs = append(f.responseStubs, resp) +} From 6732aa9725f0de73515099b650e25d26fedd853e Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 12 Feb 2020 08:48:25 -0600 Subject: [PATCH 22/51] fix tests --- command/issue_test.go | 218 ++++------------------------ command/pr_test.go | 328 ++++-------------------------------------- command/root.go | 1 + command/testing.go | 3 + 4 files changed, 61 insertions(+), 489 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index afc0d0c2e..edce87fb6 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -9,33 +9,14 @@ import ( "regexp" "testing" - "github.com/cli/cli/context" "github.com/cli/cli/utils" ) func TestIssueStatus(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("master") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) jsonFile, _ := os.Open("../test/fixtures/issueStatus.json") defer jsonFile.Close() http.StubResponse(200, jsonFile) @@ -61,28 +42,9 @@ func TestIssueStatus(t *testing.T) { } func TestIssueStatus_blankSlate(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("master") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { @@ -117,28 +79,9 @@ Issues opened by you } func TestIssueStatus_disabledIssues(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("master") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { @@ -153,28 +96,9 @@ func TestIssueStatus_disabledIssues(t *testing.T) { } func TestIssueList(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("master") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/issueList.json") defer jsonFile.Close() @@ -200,28 +124,9 @@ func TestIssueList(t *testing.T) { } func TestIssueList_withFlags(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("master") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { @@ -242,7 +147,7 @@ Issues for OWNER/REPO No issues match your search `) - bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) + bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) reqBody := struct { Variables struct { Assignee string @@ -258,28 +163,9 @@ No issues match your search } func TestIssueList_nullAssigneeLabels(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("master") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { @@ -293,7 +179,7 @@ func TestIssueList_nullAssigneeLabels(t *testing.T) { t.Errorf("error running command `issue list`: %v", err) } - bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) + bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) reqBody := struct { Variables map[string]interface{} }{} @@ -308,6 +194,7 @@ func TestIssueList_nullAssigneeLabels(t *testing.T) { func TestIssueList_disabledIssues(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { @@ -324,6 +211,7 @@ func TestIssueList_disabledIssues(t *testing.T) { func TestIssueView(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "hasIssuesEnabled": true, "issue": { @@ -357,6 +245,7 @@ func TestIssueView(t *testing.T) { func TestIssueView_preview(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "hasIssuesEnabled": true, "issue": { @@ -429,6 +318,7 @@ func TestIssueView_notFound(t *testing.T) { func TestIssueView_disabledIssues(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { @@ -446,6 +336,7 @@ func TestIssueView_disabledIssues(t *testing.T) { func TestIssueView_urlArg(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "hasIssuesEnabled": true, "issue": { @@ -476,28 +367,9 @@ func TestIssueView_urlArg(t *testing.T) { } func TestIssueCreate(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("master") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { @@ -516,7 +388,7 @@ func TestIssueCreate(t *testing.T) { t.Errorf("error running command `issue create`: %v", err) } - bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) + bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) reqBody := struct { Variables struct { Input struct { @@ -536,28 +408,9 @@ func TestIssueCreate(t *testing.T) { } func TestIssueCreate_disabledIssues(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("master") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { @@ -573,28 +426,9 @@ func TestIssueCreate_disabledIssues(t *testing.T) { } func TestIssueCreate_web(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("master") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") var seenCmd *exec.Cmd restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { diff --git a/command/pr_test.go b/command/pr_test.go index d194cf726..f0069371f 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -11,7 +11,6 @@ import ( "strings" "testing" - "github.com/cli/cli/context" "github.com/cli/cli/utils" "github.com/google/shlex" "github.com/spf13/cobra" @@ -72,28 +71,9 @@ func RunCommand(cmd *cobra.Command, args string) (*cmdOut, error) { } func TestPRStatus(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("blueberries") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prStatus.json") defer jsonFile.Close() @@ -119,28 +99,9 @@ func TestPRStatus(t *testing.T) { } func TestPRStatus_reviewsAndChecks(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("blueberries") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prStatusChecks.json") defer jsonFile.Close() @@ -165,28 +126,9 @@ func TestPRStatus_reviewsAndChecks(t *testing.T) { } func TestPRStatus_blankSlate(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("blueberries") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": {} } @@ -216,28 +158,9 @@ Requesting a code review from you } func TestPRList(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("master") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prList.json") defer jsonFile.Close() @@ -255,28 +178,9 @@ func TestPRList(t *testing.T) { } func TestPRList_filtering(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("master") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") respBody := bytes.NewBufferString(`{ "data": {} }`) http.StubResponse(200, respBody) @@ -293,7 +197,7 @@ Pull requests for OWNER/REPO No pull requests match your search `) - bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) + bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) reqBody := struct { Variables struct { State []string @@ -307,28 +211,9 @@ No pull requests match your search } func TestPRList_filteringAssignee(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("master") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") respBody := bytes.NewBufferString(`{ "data": {} }`) http.StubResponse(200, respBody) @@ -338,7 +223,7 @@ func TestPRList_filteringAssignee(t *testing.T) { t.Fatal(err) } - bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) + bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) reqBody := struct { Variables struct { Q string @@ -352,6 +237,7 @@ func TestPRList_filteringAssignee(t *testing.T) { func TestPRList_filteringAssigneeLabels(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") respBody := bytes.NewBufferString(`{ "data": {} }`) http.StubResponse(200, respBody) @@ -363,28 +249,9 @@ func TestPRList_filteringAssigneeLabels(t *testing.T) { } func TestPRView_preview(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("master") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prViewPreview.json") defer jsonFile.Close() @@ -412,28 +279,9 @@ func TestPRView_preview(t *testing.T) { } func TestPRView_previewCurrentBranch(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("blueberries") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prView.json") defer jsonFile.Close() @@ -466,28 +314,9 @@ func TestPRView_previewCurrentBranch(t *testing.T) { } func TestPRView_currentBranch(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("blueberries") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prView.json") defer jsonFile.Close() @@ -523,28 +352,9 @@ func TestPRView_currentBranch(t *testing.T) { } func TestPRView_noResultsForBranch(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("blueberries") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prView_NoActiveBranch.json") defer jsonFile.Close() @@ -573,28 +383,9 @@ func TestPRView_noResultsForBranch(t *testing.T) { } func TestPRView_numberArg(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("master") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { @@ -624,28 +415,9 @@ func TestPRView_numberArg(t *testing.T) { } func TestPRView_urlArg(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("master") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { @@ -675,28 +447,9 @@ func TestPRView_urlArg(t *testing.T) { } func TestPRView_branchArg(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("master") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequests": { "nodes": [ @@ -728,28 +481,9 @@ func TestPRView_branchArg(t *testing.T) { } func TestPRView_branchWithOwnerArg(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("master") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequests": { "nodes": [ diff --git a/command/root.go b/command/root.go index 7b9b62ce1..0864bfca7 100644 --- a/command/root.go +++ b/command/root.go @@ -173,6 +173,7 @@ func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (*ghrepo.Interfa if err != nil { return nil, err } + var baseRepo ghrepo.Interface baseRepo, err = repoContext.BaseRepo() if err != nil { diff --git a/command/testing.go b/command/testing.go index 370ba7b53..206c8517d 100644 --- a/command/testing.go +++ b/command/testing.go @@ -12,6 +12,9 @@ func initBlankContext(repo, branch string) { ctx := context.NewBlank() ctx.SetBaseRepo(repo) ctx.SetBranch(branch) + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) return ctx } } From 4f10a6d813008d3e6edb89a085fd31e4c9124f76 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 12 Feb 2020 08:57:55 -0600 Subject: [PATCH 23/51] do not use local template files if overriding base repo --- command/issue.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/command/issue.go b/command/issue.go index 0d11f5ad4..4641d91c8 100644 --- a/command/issue.go +++ b/command/issue.go @@ -287,10 +287,17 @@ func issueCreate(cmd *cobra.Command, args []string) error { fmt.Fprintf(colorableErr(cmd), "\nCreating issue in %s\n\n", ghrepo.FullName(*baseRepo)) + baseOverride, err := cmd.Flags().GetString("repo") + if err != nil { + return err + } + var templateFiles []string - if rootDir, err := git.ToplevelDir(); err == nil { - // TODO: figure out how to stub this in tests - templateFiles = githubtemplate.Find(rootDir, "ISSUE_TEMPLATE") + if baseOverride == "" { + if rootDir, err := git.ToplevelDir(); err == nil { + // TODO: figure out how to stub this in tests + templateFiles = githubtemplate.Find(rootDir, "ISSUE_TEMPLATE") + } } if isWeb, err := cmd.Flags().GetBool("web"); err == nil && isWeb { From d122f1ff175167dc5180436a60ddf21fcc778a8a Mon Sep 17 00:00:00 2001 From: Billy Griffin <5091167+billygriffin@users.noreply.github.com> Date: Wed, 12 Feb 2020 09:02:25 -0700 Subject: [PATCH 24/51] Update docs link to correct manual page --- README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/README.md b/README.md index df22d122b..7058e1f5d 100644 --- a/README.md +++ b/README.md @@ -81,8 +81,7 @@ Install a prebuilt binary from the [releases page][] ### [Build from source](/source.md) - -[docs]: https://cli.github.io/cli/manual/gh +[docs]: https://cli.github.com/manual [scoop]: https://scoop.sh [releases page]: https://github.com/cli/cli/releases/latest [hub]: https://github.com/github/hub From 2ff31909cdbe5e4c91ba696c0cc26c74b7db53a0 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 12 Feb 2020 10:34:27 -0600 Subject: [PATCH 25/51] use new helper --- command/pr_create_test.go | 69 ++++----------------------------------- 1 file changed, 6 insertions(+), 63 deletions(-) diff --git a/command/pr_create_test.go b/command/pr_create_test.go index 4a901ed3a..24d5bbd13 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -10,7 +10,6 @@ import ( "strings" "testing" - "github.com/cli/cli/context" "github.com/cli/cli/git" "github.com/cli/cli/test" "github.com/cli/cli/utils" @@ -41,28 +40,9 @@ func TestPrCreateHelperProcess(*testing.T) { } func TestPRCreate(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("feature") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "feature") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "createPullRequest": { "pullRequest": { "URL": "https://github.com/OWNER/REPO/pull/12" @@ -102,28 +82,9 @@ func TestPRCreate(t *testing.T) { } func TestPRCreate_web(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("feature") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "feature") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { @@ -144,28 +105,10 @@ func TestPRCreate_web(t *testing.T) { } func TestPRCreate_ReportsUncommittedChanges(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("feature") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "feature") http := initFakeHTTP() - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "createPullRequest": { "pullRequest": { "URL": "https://github.com/OWNER/REPO/pull/12" From b6bd044cde405ef7429530955634eb8c498a8a87 Mon Sep 17 00:00:00 2001 From: Malachi Soord Date: Wed, 12 Feb 2020 19:01:34 +0100 Subject: [PATCH 26/51] Change AUR package --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 7058e1f5d..e3a5a3ad6 100644 --- a/README.md +++ b/README.md @@ -69,10 +69,10 @@ Install and upgrade: ### Arch Linux -Arch Linux users can install from the AUR: https://aur.archlinux.org/packages/gh/ +Arch Linux users can install from the AUR: https://aur.archlinux.org/packages/github-cli/ ```bash -$ yay -S gh +$ yay -S github-cli ``` ### Other platforms From 4a5135d820e27a10ec7739d680476e7f95aea54f Mon Sep 17 00:00:00 2001 From: Ahmed El Gabri Date: Wed, 12 Feb 2020 23:07:36 +0100 Subject: [PATCH 27/51] Use if/else/if instead --- pkg/surveyext/editor.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/pkg/surveyext/editor.go b/pkg/surveyext/editor.go index 003a96e5c..8256e454d 100644 --- a/pkg/surveyext/editor.go +++ b/pkg/surveyext/editor.go @@ -25,14 +25,11 @@ var ( func init() { if runtime.GOOS == "windows" { editor = "notepad" - } - if g := os.Getenv("GIT_EDITOR"); g != "" { + } else if g := os.Getenv("GIT_EDITOR"); g != "" { editor = g - } - if v := os.Getenv("VISUAL"); v != "" { + } else if v := os.Getenv("VISUAL"); v != "" { editor = v - } - if e := os.Getenv("EDITOR"); e != "" { + } else if e := os.Getenv("EDITOR"); e != "" { editor = e } } From 212dc1da1d3e64b0b3230a65adda2f957c6b10be Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 12 Feb 2020 10:45:50 -0600 Subject: [PATCH 28/51] properly specify head ref for cross-repo/same-branch --- command/pr_create.go | 7 +++- command/pr_create_test.go | 83 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/command/pr_create.go b/command/pr_create.go index 5a844d324..7c1109b09 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -176,12 +176,17 @@ func prCreate(cmd *cobra.Command, _ []string) error { return fmt.Errorf("pull request title must not be blank") } + headRefName := headBranch + if !ghrepo.IsSame(headRemote, baseRepo) { + headRefName = fmt.Sprintf("%s:%s", headRemote.RepoOwner(), headBranch) + } + params := map[string]interface{}{ "title": title, "body": body, "draft": isDraft, "baseRefName": baseBranch, - "headRefName": headBranch, + "headRefName": headRefName, } pr, err := api.CreatePullRequest(client, baseRepo, params) diff --git a/command/pr_create_test.go b/command/pr_create_test.go index 24d5bbd13..72cbdce94 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -10,6 +10,7 @@ import ( "strings" "testing" + "github.com/cli/cli/context" "github.com/cli/cli/git" "github.com/cli/cli/test" "github.com/cli/cli/utils" @@ -131,3 +132,85 @@ Creating pull request for feature into master in OWNER/REPO `) } +func TestPRCreate_cross_repo_same_branch(t *testing.T) { + ctx := context.NewBlank() + ctx.SetBranch("default") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + "fork": "MYSELF/REPO", + }) + initContext = func() context.Context { + return ctx + } + http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID0", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "default", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "READ" + }, + "repo_001" : { + "parent": { + "id": "REPOID0", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "default", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "READ" + }, + "id": "REPOID1", + "name": "REPO", + "owner": {"login": "MYSELF"}, + "defaultBranchRef": { + "name": "default", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "createPullRequest": { "pullRequest": { + "URL": "https://github.com/OWNER/REPO/pull/12" + } } } } + `)) + + origGitCommand := git.GitCommand + git.GitCommand = test.StubExecCommand("TestPrCreateHelperProcess", "clean") + defer func() { + git.GitCommand = origGitCommand + }() + + output, err := RunCommand(prCreateCmd, `pr create -t "cross repo" -b "same branch"`) + eq(t, err, nil) + + bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) + reqBody := struct { + Variables struct { + Input struct { + RepositoryID string + Title string + Body string + BaseRefName string + HeadRefName string + } + } + }{} + json.Unmarshal(bodyBytes, &reqBody) + + eq(t, reqBody.Variables.Input.RepositoryID, "REPOID0") + eq(t, reqBody.Variables.Input.Title, "cross repo") + eq(t, reqBody.Variables.Input.Body, "same branch") + eq(t, reqBody.Variables.Input.BaseRefName, "default") + eq(t, reqBody.Variables.Input.HeadRefName, "MYSELF:default") + + eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") + + // goal: only care that gql is formatted properly +} From 8f9574be35219917eacce8e266d0fbab529337eb Mon Sep 17 00:00:00 2001 From: Anowar Islam Date: Thu, 13 Feb 2020 01:43:51 -0800 Subject: [PATCH 29/51] added fix for empty body in pr preview --- command/pr.go | 8 +++-- command/pr_test.go | 34 +++++++++++++++++++++ test/fixtures/prView_EmptyBody.json | 46 +++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 test/fixtures/prView_EmptyBody.json diff --git a/command/pr.go b/command/pr.go index 039b8c37a..d1cb7e5be 100644 --- a/command/pr.go +++ b/command/pr.go @@ -308,9 +308,11 @@ func printPrPreview(out io.Writer, pr *api.PullRequest) { pr.BaseRefName, pr.HeadRefName, ))) - fmt.Fprintln(out) - fmt.Fprintln(out, utils.RenderMarkdown(pr.Body)) - fmt.Fprintln(out) + if pr.Body != "" { + fmt.Fprintln(out) + fmt.Fprintln(out, utils.RenderMarkdown(pr.Body)) + fmt.Fprintln(out) + } fmt.Fprintf(out, utils.Gray("View this pull request on GitHub: %s\n"), pr.URL) } diff --git a/command/pr_test.go b/command/pr_test.go index f0069371f..d621d447b 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -313,6 +313,40 @@ func TestPRView_previewCurrentBranch(t *testing.T) { } } +func TestPRView_previewCurrentBranchWithEmptyBody(t *testing.T) { + initBlankContext("OWNER/REPO", "blueberries") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + jsonFile, _ := os.Open("../test/fixtures/prView_EmptyBody.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + return &outputStub{} + }) + defer restoreCmd() + + output, err := RunCommand(prViewCmd, "pr view -p") + if err != nil { + t.Errorf("error running command `pr view`: %v", err) + } + + eq(t, output.Stderr(), "") + + expectedLines := []*regexp.Regexp{ + regexp.MustCompile(`Blueberries are a good fruit`), + regexp.MustCompile(`nobody wants to merge 8 commits into master from blueberries`), + regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`), + } + for _, r := range expectedLines { + if !r.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) + return + } + } +} + func TestPRView_currentBranch(t *testing.T) { initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() diff --git a/test/fixtures/prView_EmptyBody.json b/test/fixtures/prView_EmptyBody.json new file mode 100644 index 000000000..651f49ce0 --- /dev/null +++ b/test/fixtures/prView_EmptyBody.json @@ -0,0 +1,46 @@ +{ + "data": { + "repository": { + "pullRequests": { + "nodes": [ + { + "number": 12, + "title": "Blueberries are from a fork", + "body": "yeah", + "url": "https://github.com/OWNER/REPO/pull/12", + "headRefName": "blueberries", + "baseRefName": "master", + "headRepositoryOwner": { + "login": "hubot" + }, + "commits": { + "totalCount": 12 + }, + "author": { + "login": "nobody" + }, + "isCrossRepository": true + }, + { + "number": 10, + "title": "Blueberries are a good fruit", + "body": "", + "url": "https://github.com/OWNER/REPO/pull/10", + "baseRefName": "master", + "headRefName": "blueberries", + "author": { + "login": "nobody" + }, + "headRepositoryOwner": { + "login": "OWNER" + }, + "commits": { + "totalCount": 8 + }, + "isCrossRepository": false + } + ] + } + } + } +} From 2c2c8a999148a402ce4c69ee4313b25ce885b2ad Mon Sep 17 00:00:00 2001 From: Anowar Islam Date: Thu, 13 Feb 2020 01:44:03 -0800 Subject: [PATCH 30/51] added fix for empty body in issue preview --- command/issue.go | 8 +++++--- command/issue_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/command/issue.go b/command/issue.go index 4641d91c8..6caf81c12 100644 --- a/command/issue.go +++ b/command/issue.go @@ -255,9 +255,11 @@ func printIssuePreview(out io.Writer, issue *api.Issue) { utils.Pluralize(issue.Comments.TotalCount, "comment"), coloredLabels, ))) - fmt.Fprintln(out) - fmt.Fprintln(out, utils.RenderMarkdown(issue.Body)) - fmt.Fprintln(out) + if issue.Body != "" { + fmt.Fprintln(out) + fmt.Fprintln(out, utils.RenderMarkdown(issue.Body)) + fmt.Fprintln(out) + } fmt.Fprintf(out, utils.Gray("View this issue on GitHub: %s\n"), issue.URL) } diff --git a/command/issue_test.go b/command/issue_test.go index edce87fb6..c09de2c62 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -288,6 +288,51 @@ func TestIssueView_preview(t *testing.T) { } } +func TestIssueView_previewWithEmptyBody(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 123, + "body": "", + "title": "ix of coins", + "author": { + "login": "marseilles" + }, + "labels": { + "nodes": [ + {"name": "tarot"} + ] + }, + "comments": { + "totalCount": 9 + }, + "url": "https://github.com/OWNER/REPO/issues/123" + } } } } + `)) + + output, err := RunCommand(issueViewCmd, "issue view -p 123") + if err != nil { + t.Errorf("error running command `issue view`: %v", err) + } + + eq(t, output.Stderr(), "") + + expectedLines := []*regexp.Regexp{ + regexp.MustCompile(`ix of coins`), + regexp.MustCompile(`opened by marseilles. 9 comments. \(tarot\)`), + regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), + } + for _, r := range expectedLines { + if !r.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) + return + } + } +} + func TestIssueView_notFound(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() From 1b7009087e9e3d0565fbcca6f95488a1f84bfc86 Mon Sep 17 00:00:00 2001 From: Morten Linderud Date: Thu, 13 Feb 2020 11:14:39 +0100 Subject: [PATCH 31/51] [Makefile] Support reproducible builds It's bad form to embed timestamps in binaries as it prevents reproducible builds of the resulting binary. The Reproducible Builds project defines SOURCE_DATE_EPOCH to help reproduce binaries by allowing the date format to be overridden. This patch adds support for this for GNU and BSD date. Go 1.13 introduces `-trimpath` which strips build paths from all compiled binaries. This enables people to reproduce the distributed cli binary without having to recreate the build path. https://reproducible-builds.org/specs/source-date-epoch/ https://reproducible-builds.org/docs/build-path/ Signed-off-by: Morten Linderud --- Makefile | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index be2483cdc..3b8aba5ea 100644 --- a/Makefile +++ b/Makefile @@ -2,15 +2,21 @@ BUILD_FILES = $(shell go list -f '{{range .GoFiles}}{{$$.Dir}}/{{.}}\ {{end}}' ./...) GH_VERSION ?= $(shell git describe --tags 2>/dev/null || git rev-parse --short HEAD) +DATE_FMT = +%Y-%m-%d +ifdef SOURCE_DATE_EPOCH + BUILD_DATE ?= $(shell date -u -d "@$(SOURCE_DATE_EPOCH)" "$(DATE_FMT)" 2>/dev/null || date -u -r "$(SOURCE_DATE_EPOCH)" "$(DATE_FMT)" 2>/dev/null || date -u "$(DATE_FMT)") +else + BUILD_DATE ?= $(shell date "$(DATE_FMT)") +endif LDFLAGS := -X github.com/cli/cli/command.Version=$(GH_VERSION) $(LDFLAGS) -LDFLAGS := -X github.com/cli/cli/command.BuildDate=$(shell date +%Y-%m-%d) $(LDFLAGS) +LDFLAGS := -X github.com/cli/cli/command.BuildDate=$(BUILD_DATE) $(LDFLAGS) ifdef GH_OAUTH_CLIENT_SECRET LDFLAGS := -X github.com/cli/cli/context.oauthClientID=$(GH_OAUTH_CLIENT_ID) $(LDFLAGS) LDFLAGS := -X github.com/cli/cli/context.oauthClientSecret=$(GH_OAUTH_CLIENT_SECRET) $(LDFLAGS) endif bin/gh: $(BUILD_FILES) - @go build -ldflags "$(LDFLAGS)" -o "$@" ./cmd/gh + @go build -trimpath -ldflags "$(LDFLAGS)" -o "$@" ./cmd/gh test: go test ./... From 26159ddcb3e256743a990715c94b823bf804a3f4 Mon Sep 17 00:00:00 2001 From: Billy Griffin <5091167+billygriffin@users.noreply.github.com> Date: Thu, 13 Feb 2020 07:52:09 -0700 Subject: [PATCH 32/51] Add availability section to readme --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index e3a5a3ad6..01e367902 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,10 @@ the terminal next to where you are already working with `git` and your code. ![screenshot](https://user-images.githubusercontent.com/98482/73286699-9f922180-41bd-11ea-87c9-60a2d31fd0ac.png) +## Availability + +While in beta, GitHub CLI is available for repos hosted on GitHub.com only. It does not currently support repositories hosted on GitHub Enterprise Server or other hosting providers. + ## We need your feedback GitHub CLI is currently early in its development, and we're hoping to get feedback from people using it. From 12887db9c8e380e6b4c9709dd3e1c04af240bb22 Mon Sep 17 00:00:00 2001 From: Odin Ugedal Date: Thu, 13 Feb 2020 17:29:07 +0100 Subject: [PATCH 33/51] Fix wrong repo url being used for pushing afaik this should be finalURL (the fork) and not initURL (the upstream), otherwise a bit of the code above can be removed. --- git/remote.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/remote.go b/git/remote.go index 0df077efb..f98606a2e 100644 --- a/git/remote.go +++ b/git/remote.go @@ -91,7 +91,7 @@ func AddRemote(name, initURL, finalURL string) (*Remote, error) { } } - finalURLParsed, err := url.Parse(initURL) + finalURLParsed, err := url.Parse(finalURL) if err != nil { return nil, err } From 95cbc56dec810009e4eb5156995d936f6d864c3d Mon Sep 17 00:00:00 2001 From: vertextau <43003542+vertextau@users.noreply.github.com> Date: Thu, 13 Feb 2020 22:42:58 +0100 Subject: [PATCH 34/51] Improve randomString func --- auth/oauth.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/auth/oauth.go b/auth/oauth.go index 54bd5137b..6e85575bf 100644 --- a/auth/oauth.go +++ b/auth/oauth.go @@ -2,6 +2,7 @@ package auth import ( "crypto/rand" + "encoding/hex" "errors" "fmt" "io" @@ -20,7 +21,7 @@ func randomString(length int) (string, error) { if err != nil { return "", err } - return fmt.Sprintf("%x", b), nil + return hex.EncodeToString(b), nil } // OAuthFlow represents the setup for authenticating with GitHub From c5a511a2d16bba664826120854cadfadcbb46a1b Mon Sep 17 00:00:00 2001 From: vilmibm Date: Thu, 13 Feb 2020 17:16:53 -0600 Subject: [PATCH 35/51] Revert "prefix branch names for local pr checkout" This reverts commit 5b8d7c1841099612d94129cbc1e81505d3ad778d. --- command/pr_checkout.go | 3 +- command/pr_checkout_test.go | 58 ++++++++++++++++++------------------- 2 files changed, 30 insertions(+), 31 deletions(-) diff --git a/command/pr_checkout.go b/command/pr_checkout.go index 0c41dc793..43556c67f 100644 --- a/command/pr_checkout.go +++ b/command/pr_checkout.go @@ -40,8 +40,7 @@ func prCheckout(cmd *cobra.Command, args []string) error { cmdQueue := [][]string{} - // namespace PR checkout branches to avoid local branch name collisions - newBranchName := fmt.Sprintf("pr/%d/%s", pr.Number, pr.HeadRefName) + newBranchName := pr.HeadRefName if headRemote != nil { // there is an existing git remote for PR head remoteBranch := fmt.Sprintf("%s/%s", headRemote.Name, pr.HeadRefName) diff --git a/command/pr_checkout_test.go b/command/pr_checkout_test.go index 9500d4530..ba0ddff22 100644 --- a/command/pr_checkout_test.go +++ b/command/pr_checkout_test.go @@ -42,7 +42,7 @@ func TestPRCheckout_sameRepo(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { switch strings.Join(cmd.Args, " ") { - case "git show-ref --verify --quiet refs/heads/pr/123/feature": + case "git show-ref --verify --quiet refs/heads/feature": return &errorStub{"exit status: 1"} default: ranCommands = append(ranCommands, cmd.Args) @@ -57,9 +57,9 @@ func TestPRCheckout_sameRepo(t *testing.T) { eq(t, len(ranCommands), 4) eq(t, strings.Join(ranCommands[0], " "), "git fetch origin +refs/heads/feature:refs/remotes/origin/feature") - eq(t, strings.Join(ranCommands[1], " "), "git checkout -b pr/123/feature --no-track origin/feature") - eq(t, strings.Join(ranCommands[2], " "), "git config branch.pr/123/feature.remote origin") - eq(t, strings.Join(ranCommands[3], " "), "git config branch.pr/123/feature.merge refs/heads/feature") + eq(t, strings.Join(ranCommands[1], " "), "git checkout -b feature --no-track origin/feature") + eq(t, strings.Join(ranCommands[2], " "), "git config branch.feature.remote origin") + eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.merge refs/heads/feature") } func TestPRCheckout_urlArg(t *testing.T) { @@ -94,7 +94,7 @@ func TestPRCheckout_urlArg(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { switch strings.Join(cmd.Args, " ") { - case "git show-ref --verify --quiet refs/heads/pr/123/feature": + case "git show-ref --verify --quiet refs/heads/feature": return &errorStub{"exit status: 1"} default: ranCommands = append(ranCommands, cmd.Args) @@ -108,7 +108,7 @@ func TestPRCheckout_urlArg(t *testing.T) { eq(t, output.String(), "") eq(t, len(ranCommands), 4) - eq(t, strings.Join(ranCommands[1], " "), "git checkout -b pr/123/feature --no-track origin/feature") + eq(t, strings.Join(ranCommands[1], " "), "git checkout -b feature --no-track origin/feature") } func TestPRCheckout_branchArg(t *testing.T) { @@ -143,7 +143,7 @@ func TestPRCheckout_branchArg(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { switch strings.Join(cmd.Args, " ") { - case "git show-ref --verify --quiet refs/heads/pr/123/feature": + case "git show-ref --verify --quiet refs/heads/feature": return &errorStub{"exit status: 1"} default: ranCommands = append(ranCommands, cmd.Args) @@ -157,7 +157,7 @@ func TestPRCheckout_branchArg(t *testing.T) { eq(t, output.String(), "") eq(t, len(ranCommands), 5) - eq(t, strings.Join(ranCommands[1], " "), "git fetch origin refs/pull/123/head:pr/123/feature") + eq(t, strings.Join(ranCommands[1], " "), "git fetch origin refs/pull/123/head:feature") } func TestPRCheckout_existingBranch(t *testing.T) { @@ -192,7 +192,7 @@ func TestPRCheckout_existingBranch(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { switch strings.Join(cmd.Args, " ") { - case "git show-ref --verify --quiet refs/heads/pr/123/feature": + case "git show-ref --verify --quiet refs/heads/feature": return &outputStub{} default: ranCommands = append(ranCommands, cmd.Args) @@ -207,7 +207,7 @@ func TestPRCheckout_existingBranch(t *testing.T) { eq(t, len(ranCommands), 3) eq(t, strings.Join(ranCommands[0], " "), "git fetch origin +refs/heads/feature:refs/remotes/origin/feature") - eq(t, strings.Join(ranCommands[1], " "), "git checkout pr/123/feature") + eq(t, strings.Join(ranCommands[1], " "), "git checkout feature") eq(t, strings.Join(ranCommands[2], " "), "git merge --ff-only refs/remotes/origin/feature") } @@ -244,7 +244,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { switch strings.Join(cmd.Args, " ") { - case "git show-ref --verify --quiet refs/heads/pr/123/feature": + case "git show-ref --verify --quiet refs/heads/feature": return &errorStub{"exit status: 1"} default: ranCommands = append(ranCommands, cmd.Args) @@ -259,9 +259,9 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) { eq(t, len(ranCommands), 4) eq(t, strings.Join(ranCommands[0], " "), "git fetch robot-fork +refs/heads/feature:refs/remotes/robot-fork/feature") - eq(t, strings.Join(ranCommands[1], " "), "git checkout -b pr/123/feature --no-track robot-fork/feature") - eq(t, strings.Join(ranCommands[2], " "), "git config branch.pr/123/feature.remote robot-fork") - eq(t, strings.Join(ranCommands[3], " "), "git config branch.pr/123/feature.merge refs/heads/feature") + eq(t, strings.Join(ranCommands[1], " "), "git checkout -b feature --no-track robot-fork/feature") + eq(t, strings.Join(ranCommands[2], " "), "git config branch.feature.remote robot-fork") + eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.merge refs/heads/feature") } func TestPRCheckout_differentRepo(t *testing.T) { @@ -296,7 +296,7 @@ func TestPRCheckout_differentRepo(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { switch strings.Join(cmd.Args, " ") { - case "git config branch.pr/123/feature.merge": + case "git config branch.feature.merge": return &errorStub{"exit status 1"} default: ranCommands = append(ranCommands, cmd.Args) @@ -310,10 +310,10 @@ func TestPRCheckout_differentRepo(t *testing.T) { eq(t, output.String(), "") eq(t, len(ranCommands), 4) - eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:pr/123/feature") - eq(t, strings.Join(ranCommands[1], " "), "git checkout pr/123/feature") - eq(t, strings.Join(ranCommands[2], " "), "git config branch.pr/123/feature.remote origin") - eq(t, strings.Join(ranCommands[3], " "), "git config branch.pr/123/feature.merge refs/pull/123/head") + eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:feature") + eq(t, strings.Join(ranCommands[1], " "), "git checkout feature") + eq(t, strings.Join(ranCommands[2], " "), "git config branch.feature.remote origin") + eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.merge refs/pull/123/head") } func TestPRCheckout_differentRepo_existingBranch(t *testing.T) { @@ -348,7 +348,7 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { switch strings.Join(cmd.Args, " ") { - case "git config branch.pr/123/feature.merge": + case "git config branch.feature.merge": return &outputStub{[]byte("refs/heads/feature\n")} default: ranCommands = append(ranCommands, cmd.Args) @@ -362,13 +362,13 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) { eq(t, output.String(), "") eq(t, len(ranCommands), 2) - eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:pr/123/feature") - eq(t, strings.Join(ranCommands[1], " "), "git checkout pr/123/feature") + eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:feature") + eq(t, strings.Join(ranCommands[1], " "), "git checkout feature") } func TestPRCheckout_differentRepo_currentBranch(t *testing.T) { ctx := context.NewBlank() - ctx.SetBranch("pr/123/feature") + ctx.SetBranch("feature") ctx.SetRemotes(map[string]string{ "origin": "OWNER/REPO", }) @@ -398,7 +398,7 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { switch strings.Join(cmd.Args, " ") { - case "git config branch.pr/123/feature.merge": + case "git config branch.feature.merge": return &outputStub{[]byte("refs/heads/feature\n")} default: ranCommands = append(ranCommands, cmd.Args) @@ -448,7 +448,7 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { switch strings.Join(cmd.Args, " ") { - case "git config branch.pr/123/feature.merge": + case "git config branch.feature.merge": return &errorStub{"exit status 1"} default: ranCommands = append(ranCommands, cmd.Args) @@ -462,8 +462,8 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) { eq(t, output.String(), "") eq(t, len(ranCommands), 4) - eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:pr/123/feature") - eq(t, strings.Join(ranCommands[1], " "), "git checkout pr/123/feature") - eq(t, strings.Join(ranCommands[2], " "), "git config branch.pr/123/feature.remote https://github.com/hubot/REPO.git") - eq(t, strings.Join(ranCommands[3], " "), "git config branch.pr/123/feature.merge refs/heads/feature") + eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:feature") + eq(t, strings.Join(ranCommands[1], " "), "git checkout feature") + eq(t, strings.Join(ranCommands[2], " "), "git config branch.feature.remote https://github.com/hubot/REPO.git") + eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.merge refs/heads/feature") } From 5c8dd70bb7e00d613e36ec255869e8790fd03e78 Mon Sep 17 00:00:00 2001 From: TaKO8Ki Date: Sat, 15 Feb 2020 23:20:15 +0900 Subject: [PATCH 36/51] fix typo --- update/update.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/update/update.go b/update/update.go index 42eb5955f..5a96bed56 100644 --- a/update/update.go +++ b/update/update.go @@ -21,7 +21,7 @@ type StateEntry struct { LatestRelease ReleaseInfo `yaml:"latest_release"` } -// CheckForUpdate checks whether this software has had a newer relase on GitHub +// CheckForUpdate checks whether this software has had a newer release on GitHub func CheckForUpdate(client *api.Client, stateFilePath, repo, currentVersion string) (*ReleaseInfo, error) { latestRelease, err := getLatestReleaseInfo(client, stateFilePath, repo, currentVersion) if err != nil { From 71b1c492274d10aef975a283499a31b478a38df2 Mon Sep 17 00:00:00 2001 From: Gary Ewan Park Date: Sat, 15 Feb 2020 15:19:24 +0000 Subject: [PATCH 37/51] (doc) Update Windows installation options To include installing/upgrading from Chocolatey --- README.md | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 01e367902..34374d6f0 100644 --- a/README.md +++ b/README.md @@ -44,7 +44,9 @@ Upgrade: `brew update && brew upgrade gh` ### Windows -`gh` is available via [scoop][]: +`gh` is available via [scoop][], [Chocolatey][], and as signed MSI's: + +#### scoop Install: @@ -55,6 +57,22 @@ scoop install gh Upgrade: `scoop update gh` +#### Chocolatey + +Install: + +``` +choco install gh +``` + +Upgrade: + +``` +choco upgrade gh +``` + +#### Signed MSI + Signed MSI installers are also available on the [releases page][]. ### Debian/Ubuntu Linux @@ -87,6 +105,7 @@ Install a prebuilt binary from the [releases page][] [docs]: https://cli.github.com/manual [scoop]: https://scoop.sh +[Chocolatey]: https://chocolatey.org [releases page]: https://github.com/cli/cli/releases/latest [hub]: https://github.com/github/hub [contributing page]: https://github.com/cli/cli/blob/master/.github/CONTRIBUTING.md From 3075af84d97bf0b1b78afcc0dd994a008eabd0bb Mon Sep 17 00:00:00 2001 From: Emerson Castaneda Date: Sat, 15 Feb 2020 16:08:54 -0500 Subject: [PATCH 38/51] Add Go ver. note for installing from source --- source.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/source.md b/source.md index da27f29fe..3e13f002f 100644 --- a/source.md +++ b/source.md @@ -10,6 +10,8 @@ $ git clone https://github.com/cli/cli.git ~/.githubcli $ cd ~/.githubcli && make ``` +_Note: it requires Go 1.13+_ + 3. Add `~/.githubcli/bin` to your $PATH for access to the gh command-line utility. * For **bash**: From b67bb5538345efc45627f1463e8096326aaf410f Mon Sep 17 00:00:00 2001 From: Dmitry Sharshakov Date: Sun, 16 Feb 2020 17:28:03 +0300 Subject: [PATCH 39/51] Correct typoes --- command/pr.go | 2 +- update/update.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/command/pr.go b/command/pr.go index d1cb7e5be..1e0c43aa3 100644 --- a/command/pr.go +++ b/command/pr.go @@ -193,7 +193,7 @@ func prList(cmd *cobra.Command, args []string) error { } if len(prs) == 0 { - colorErr := colorableErr(cmd) // Send to stderr because otherwise when piping this command it would seem like the "no open prs" message is acually a pr + colorErr := colorableErr(cmd) // Send to stderr because otherwise when piping this command it would seem like the "no open prs" message is actually a pr msg := "There are no open pull requests" userSetFlags := false diff --git a/update/update.go b/update/update.go index 42eb5955f..5a96bed56 100644 --- a/update/update.go +++ b/update/update.go @@ -21,7 +21,7 @@ type StateEntry struct { LatestRelease ReleaseInfo `yaml:"latest_release"` } -// CheckForUpdate checks whether this software has had a newer relase on GitHub +// CheckForUpdate checks whether this software has had a newer release on GitHub func CheckForUpdate(client *api.Client, stateFilePath, repo, currentVersion string) (*ReleaseInfo, error) { latestRelease, err := getLatestReleaseInfo(client, stateFilePath, repo, currentVersion) if err != nil { From 101fbc27e1c5a0953b5a9dc4695a298e11bd2966 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 17 Feb 2020 10:37:14 +0100 Subject: [PATCH 40/51] Reword Windows install instructions --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 34374d6f0..04d00bd74 100644 --- a/README.md +++ b/README.md @@ -44,7 +44,7 @@ Upgrade: `brew update && brew upgrade gh` ### Windows -`gh` is available via [scoop][], [Chocolatey][], and as signed MSI's: +`gh` is available via [scoop][], [Chocolatey][], and as downloadable MSI. #### scoop @@ -73,7 +73,7 @@ choco upgrade gh #### Signed MSI -Signed MSI installers are also available on the [releases page][]. +MSI installers are available for download on the [releases page][]. ### Debian/Ubuntu Linux From 3138b3b2a634f29c55eb374e17f557fcfcbc0b7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 17 Feb 2020 12:07:39 +0100 Subject: [PATCH 41/51] Update note about required Go version --- source.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/source.md b/source.md index 3e13f002f..75d1e1c5a 100644 --- a/source.md +++ b/source.md @@ -1,5 +1,11 @@ # Installation from source +0. Verify that you have Go 1.13+ installed +``` +$ go version +go version go1.13.7 +``` + 1. Clone cli into `~/.githubcli` ``` $ git clone https://github.com/cli/cli.git ~/.githubcli @@ -10,8 +16,6 @@ $ git clone https://github.com/cli/cli.git ~/.githubcli $ cd ~/.githubcli && make ``` -_Note: it requires Go 1.13+_ - 3. Add `~/.githubcli/bin` to your $PATH for access to the gh command-line utility. * For **bash**: From 5f6fd35a99dd09b8fe0660589846209607af704e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 17 Feb 2020 12:52:48 +0100 Subject: [PATCH 42/51] Enable CI for pull requests from forks --- .github/workflows/go.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 9800d2827..557f87049 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -1,5 +1,5 @@ name: Tests -on: [push] +on: [push, pull_request] jobs: build: strategy: From 69d859a47f7b6f664eb4fabf4b82c647a9b2fc26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 17 Feb 2020 15:43:51 +0100 Subject: [PATCH 43/51] Add labels to issue template --- .github/ISSUE_TEMPLATE/bug_report.md | 3 +++ .github/ISSUE_TEMPLATE/submit-a-request.md | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 19e727fdf..16e5348f1 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -1,6 +1,9 @@ --- name: "\U0001F41B Bug report" about: Report a bug or unexpected behavior while using GitHub CLI +title: '' +labels: bug +assignees: '' --- diff --git a/.github/ISSUE_TEMPLATE/submit-a-request.md b/.github/ISSUE_TEMPLATE/submit-a-request.md index 585afa21e..4f66ac457 100644 --- a/.github/ISSUE_TEMPLATE/submit-a-request.md +++ b/.github/ISSUE_TEMPLATE/submit-a-request.md @@ -1,6 +1,9 @@ --- -name: "\U00002B50 Submit a request" +name: "⭐ Submit a request" about: Surface a feature or problem that you think should be solved +title: '' +labels: enhancement +assignees: '' --- From 103d62b309632aa33ca79934b811660188be133d Mon Sep 17 00:00:00 2001 From: Emerson Castaneda Date: Mon, 17 Feb 2020 21:28:51 -0500 Subject: [PATCH 44/51] Add manual-pages-are-automatically-generated clarification --- .github/CONTRIBUTING.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 381d4b0ce..e5ad39f5c 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -26,6 +26,8 @@ Contributions to this project are [released][legal] to the public under the [pro Please note that this project adheres to a [Contributor Code of Conduct][code-of-conduct]. By participating in this project you agree to abide by its terms. +We generate manual pages from source on every release! You do not need to submit PRs for those specifically; the docs will get updated if your PR gets accepted. + ## Resources - [How to Contribute to Open Source](https://opensource.guide/how-to-contribute/) From abfc4d3b1af3db4df4baad7cc10f5bd6986e4bb0 Mon Sep 17 00:00:00 2001 From: Joshua Schmid Date: Tue, 18 Feb 2020 08:53:37 +0100 Subject: [PATCH 45/51] Add openSUSE/SUSE install instructions to README --- README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/README.md b/README.md index 04d00bd74..c98b207d0 100644 --- a/README.md +++ b/README.md @@ -89,6 +89,13 @@ Install and upgrade: 1. Download the `.rpm` file from the [releases page][] 2. `sudo yum localinstall gh_*_linux_amd64.rpm` install the downloaded file +### openSUSE/SUSE Linux + +Install and upgrade: + +1. Download the `.rpm` file from the [releases page][] +2. `sudo zypper in gh_*_linux_amd64.rpm` install the downloaded file + ### Arch Linux Arch Linux users can install from the AUR: https://aur.archlinux.org/packages/github-cli/ From de6e99aa75c0d2684f93d4d3c5fa9a8d529db75a Mon Sep 17 00:00:00 2001 From: Colin Arnott Date: Tue, 18 Feb 2020 07:55:16 +0000 Subject: [PATCH 46/51] command: default to toolchain version Since the Go toolchain is able to extract the module version at build time, we should use that as a default instead of DEV. This means customers installing via go-get will get the correct version. Unfortunately, the toolchain does not store when the build occurs, so BuildTime now defaults to the empty string. It is still set if build officially, just not for go-get. --- command/root.go | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/command/root.go b/command/root.go index 0864bfca7..21b26778b 100644 --- a/command/root.go +++ b/command/root.go @@ -5,6 +5,7 @@ import ( "io" "os" "regexp" + "runtime/debug" "strings" "github.com/cli/cli/api" @@ -15,16 +16,26 @@ import ( "github.com/spf13/cobra" ) -// Version is dynamically set at build time in the Makefile -var Version = "DEV" +// Version is dynamically set by the toolchain or overriden by the Makefile. +var Version = func(info *debug.BuildInfo, ok bool) string { + if !ok { + return "(devel)" + } + return info.Main.Version +}(debug.ReadBuildInfo()) -// BuildDate is dynamically set at build time in the Makefile -var BuildDate = "YYYY-MM-DD" +// BuildDate is dynamically set at build time in the Makefile. +var BuildDate = "" // YYYY-MM-DD var versionOutput = "" func init() { - RootCmd.Version = fmt.Sprintf("%s (%s)", strings.TrimPrefix(Version, "v"), BuildDate) + Version = strings.TrimPrefix(info.Main.Version, "v") + if BuildDate == "" { + RootCmd.Version = Version + } else { + RootCmd.Version = fmt.Sprintf("%s (%s)", Version, BuildDate) + } versionOutput = fmt.Sprintf("gh version %s\n%s\n", RootCmd.Version, changelogURL(Version)) RootCmd.AddCommand(versionCmd) RootCmd.SetVersionTemplate(versionOutput) From 408b565fd96064876b1107b0dfc34d054a5c170b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 18 Feb 2020 13:47:57 +0100 Subject: [PATCH 47/51] Allow setting version via ldflags again --- command/root.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/command/root.go b/command/root.go index 21b26778b..4f9d5ba09 100644 --- a/command/root.go +++ b/command/root.go @@ -17,12 +17,7 @@ import ( ) // Version is dynamically set by the toolchain or overriden by the Makefile. -var Version = func(info *debug.BuildInfo, ok bool) string { - if !ok { - return "(devel)" - } - return info.Main.Version -}(debug.ReadBuildInfo()) +var Version = "DEV" // BuildDate is dynamically set at build time in the Makefile. var BuildDate = "" // YYYY-MM-DD @@ -30,7 +25,12 @@ var BuildDate = "" // YYYY-MM-DD var versionOutput = "" func init() { - Version = strings.TrimPrefix(info.Main.Version, "v") + if Version == "DEV" { + if info, ok := debug.ReadBuildInfo(); ok && info.Main.Version != "(devel)" { + Version = info.Main.Version + } + } + Version = strings.TrimPrefix(Version, "v") if BuildDate == "" { RootCmd.Version = Version } else { From 213f4a5333c21020da030012979bd326a34fb28c Mon Sep 17 00:00:00 2001 From: Colin Arnott Date: Tue, 18 Feb 2020 08:00:18 +0000 Subject: [PATCH 48/51] context: use the real oauth credentials It is trivial to extract this information from the released artefacts, thus there is no security benefit to the safe/development credentials. This approach also prevents users from using go-get to install. As proof of concept, and to enable go-get, this change embeds the GitHub CLI credentials, instead of GitHub CLI (dev). --- context/config_setup.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/context/config_setup.go b/context/config_setup.go index 4d91a4c62..4236a81d9 100644 --- a/context/config_setup.go +++ b/context/config_setup.go @@ -18,10 +18,8 @@ const ( ) var ( - // The GitHub app that is meant for development - oauthClientID = "4d747ba5675d5d66553f" - // This value is safe to be embedded in version control - oauthClientSecret = "d4fee7b3f9c2ef4284a5ca7be0ee200cf15b6f8d" + oauthClientID = "178c6fc778ccc68e1d6a" + oauthClientSecret = "34ddeff2b558a23d38fba8a6de74f086ede1cc0b" ) // TODO: have a conversation about whether this belongs in the "context" package From 72bde685b06271e5bc4288ea034c91207d4d901f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 18 Feb 2020 19:22:01 +0100 Subject: [PATCH 49/51] Add back comments explaining OAuth app credentials --- context/config_setup.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/context/config_setup.go b/context/config_setup.go index 4236a81d9..6cb8ddb6c 100644 --- a/context/config_setup.go +++ b/context/config_setup.go @@ -18,7 +18,9 @@ const ( ) var ( - oauthClientID = "178c6fc778ccc68e1d6a" + // The "GitHub CLI" OAuth app + oauthClientID = "178c6fc778ccc68e1d6a" + // This value is safe to be embedded in version control oauthClientSecret = "34ddeff2b558a23d38fba8a6de74f086ede1cc0b" ) From c32bcee4bb4216ad3c749d314e93add6b5863c2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 18 Feb 2020 19:23:37 +0100 Subject: [PATCH 50/51] No need to configure production OAuth app on release anymore --- .github/workflows/releases.yml | 2 -- .goreleaser.yml | 2 -- 2 files changed, 4 deletions(-) diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml index e8a3c0143..f241dc1d9 100644 --- a/.github/workflows/releases.yml +++ b/.github/workflows/releases.yml @@ -25,8 +25,6 @@ jobs: version: latest args: release --release-notes=CHANGELOG.md env: - GH_OAUTH_CLIENT_ID: 178c6fc778ccc68e1d6a - GH_OAUTH_CLIENT_SECRET: ${{secrets.OAUTH_CLIENT_SECRET}} GITHUB_TOKEN: ${{secrets.UPLOAD_GITHUB_TOKEN}} msi: needs: goreleaser diff --git a/.goreleaser.yml b/.goreleaser.yml index 1eb611313..619180def 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -13,8 +13,6 @@ builds: main: ./cmd/gh ldflags: - -s -w -X github.com/cli/cli/command.Version={{.Version}} -X github.com/cli/cli/command.BuildDate={{time "2006-01-02"}} - - -X github.com/cli/cli/context.oauthClientID={{.Env.GH_OAUTH_CLIENT_ID}} - - -X github.com/cli/cli/context.oauthClientSecret={{.Env.GH_OAUTH_CLIENT_SECRET}} - -X main.updaterEnabled=cli/cli id: macos goos: [darwin] From 61ff5d73bd899308f733e1d3b0168fd7df8e77ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 18 Feb 2020 20:03:09 +0100 Subject: [PATCH 51/51] Fix crash in `issue/pr list` for narrow terminals If the available column width is smaller than 3, don't try to truncate with ellipsis (`...`). Instead, just truncate to available width. --- utils/table_printer.go | 5 ++++- utils/table_printer_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 utils/table_printer_test.go diff --git a/utils/table_printer.go b/utils/table_printer.go index 649e246e1..e1bf9bc36 100644 --- a/utils/table_printer.go +++ b/utils/table_printer.go @@ -176,7 +176,10 @@ func (t *tsvTablePrinter) Render() error { func truncate(maxLength int, title string) string { if len(title) > maxLength { - return title[0:maxLength-3] + "..." + if maxLength > 3 { + return title[0:maxLength-3] + "..." + } + return title[0:maxLength] } return title } diff --git a/utils/table_printer_test.go b/utils/table_printer_test.go new file mode 100644 index 000000000..2e3e4e803 --- /dev/null +++ b/utils/table_printer_test.go @@ -0,0 +1,31 @@ +package utils + +import ( + "bytes" + "testing" +) + +func Test_ttyTablePrinter_truncate(t *testing.T) { + buf := bytes.Buffer{} + tp := &ttyTablePrinter{ + out: &buf, + maxWidth: 5, + } + + tp.AddField("1", nil, nil) + tp.AddField("hello", nil, nil) + tp.EndRow() + tp.AddField("2", nil, nil) + tp.AddField("world", nil, nil) + tp.EndRow() + + err := tp.Render() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + expected := "1 he\n2 wo\n" + if buf.String() != expected { + t.Errorf("expected: %q, got: %q", expected, buf.String()) + } +}