From 569321b48dafb3cf233714b8ea092a169fe3ba26 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 12 Nov 2019 11:31:43 -0800 Subject: [PATCH 01/28] Updated README.md --- README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/README.md b/README.md index ca23a060d..5ada69ed2 100644 --- a/README.md +++ b/README.md @@ -9,3 +9,11 @@ This tool is an endeavor separate from [github/hub](https://github.com/github/hu - [Demo planning doc](https://docs.google.com/document/d/18ym-_xjFTSXe0-xzgaBn13Su7MEhWfLE5qSNPJV4M0A/edit) - [Weekly tracking issue](https://github.com/github/gh-cli/labels/tracking%20issue) - [Weekly sync notes](https://docs.google.com/document/d/1eUo9nIzXbC1DG26Y3dk9hOceLua2yFlwlvFPZ82MwHg/edit) + +# How to create a release + +This can all be done from your local terminal. + +1. `git tag -a vYOUR_VERSION_NUMBER -m 'vYOUR_VERSION_NUMBER'` +2. `git push origin vYOUR_VERSION_NUMBER` +3. Go to https://github.com/github/homebrew-gh/releases and look at the release (read https://github.com/github/homebrew-gh for how to install the release via homebrew) \ No newline at end of file From 44c7495bab84283fefc026030a4dbae3b319bb21 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 12 Nov 2019 14:31:24 -0800 Subject: [PATCH 02/28] Add state --- api/queries.go | 104 ---------------------- api/queries_issue.go | 164 +++++++++++++++++++++++++++++++++++ command/issue.go | 45 +++++++++- command/issue_test.go | 26 ++++++ test/fixtures/issueList.json | 58 ++++--------- 5 files changed, 252 insertions(+), 145 deletions(-) diff --git a/api/queries.go b/api/queries.go index 5daf10244..68a9d14ba 100644 --- a/api/queries.go +++ b/api/queries.go @@ -2,7 +2,6 @@ package api import ( "fmt" - "time" ) type PullRequestsPayload struct { @@ -24,109 +23,6 @@ type Repo interface { RepoOwner() string } -type IssuesPayload struct { - Assigned []Issue - Mentioned []Issue - Recent []Issue -} - -type Issue struct { - Number int - Title string - URL string -} - -func Issues(client *Client, ghRepo Repo, currentUsername string) (*IssuesPayload, error) { - type issues struct { - Issues struct { - Edges []struct { - Node Issue - } - } - } - - type response struct { - Assigned issues - Mentioned issues - Recent issues - } - - query := ` - fragment issue on Issue { - number - title - } - query($owner: String!, $repo: String!, $since: DateTime!, $viewer: String!, $per_page: Int = 10) { - assigned: repository(owner: $owner, name: $repo) { - issues(filterBy: {assignee: $viewer}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { - edges { - node { - ...issue - } - } - } - } - mentioned: repository(owner: $owner, name: $repo) { - issues(filterBy: {mentioned: $viewer}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { - edges { - node { - ...issue - } - } - } - } - recent: repository(owner: $owner, name: $repo) { - issues(filterBy: {since: $since}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { - edges { - node { - ...issue - } - } - } - } - } - ` - - owner := ghRepo.RepoOwner() - repo := ghRepo.RepoName() - since := time.Now().UTC().Add(time.Hour * -24).Format("2006-01-02T15:04:05-0700") - variables := map[string]interface{}{ - "owner": owner, - "repo": repo, - "viewer": currentUsername, - "since": since, - } - - var resp response - err := client.GraphQL(query, variables, &resp) - if err != nil { - return nil, err - } - - var assigned []Issue - for _, edge := range resp.Assigned.Issues.Edges { - assigned = append(assigned, edge.Node) - } - - var mentioned []Issue - for _, edge := range resp.Mentioned.Issues.Edges { - mentioned = append(mentioned, edge.Node) - } - - var recent []Issue - for _, edge := range resp.Recent.Issues.Edges { - recent = append(recent, edge.Node) - } - - payload := IssuesPayload{ - assigned, - mentioned, - recent, - } - - return &payload, nil -} - func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername string) (*PullRequestsPayload, error) { type edges struct { Edges []struct { diff --git a/api/queries_issue.go b/api/queries_issue.go index 56a235c89..96d0e1b2b 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -1,5 +1,10 @@ package api +import ( + "fmt" + "time" +) + func IssueCreate(client *Client, ghRepo Repo, params map[string]interface{}) (*Issue, error) { repoID, err := GitHubRepoId(client, ghRepo) if err != nil { @@ -38,3 +43,162 @@ func IssueCreate(client *Client, ghRepo Repo, params map[string]interface{}) (*I return &result.CreateIssue.Issue, nil } + +type IssuesPayload struct { + Assigned []Issue + Mentioned []Issue + Recent []Issue +} + +type Issue struct { + Number int + Title string + URL string +} + +type apiIssues struct { + Issues struct { + Edges []struct { + Node Issue + } + } +} + +func IssueStatus(client *Client, ghRepo Repo, currentUsername string) (*IssuesPayload, error) { + type response struct { + Assigned apiIssues + Mentioned apiIssues + Recent apiIssues + } + + query := ` + fragment issue on Issue { + number + title + } + query($owner: String!, $repo: String!, $since: DateTime!, $viewer: String!, $per_page: Int = 10) { + assigned: repository(owner: $owner, name: $repo) { + issues(filterBy: {assignee: $viewer}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { + edges { + node { + ...issue + } + } + } + } + mentioned: repository(owner: $owner, name: $repo) { + issues(filterBy: {mentioned: $viewer}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { + edges { + node { + ...issue + } + } + } + } + recent: repository(owner: $owner, name: $repo) { + issues(filterBy: {since: $since}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { + edges { + node { + ...issue + } + } + } + } + } + ` + + owner := ghRepo.RepoOwner() + repo := ghRepo.RepoName() + since := time.Now().UTC().Add(time.Hour * -24).Format("2006-01-02T15:04:05-0700") + variables := map[string]interface{}{ + "owner": owner, + "repo": repo, + "viewer": currentUsername, + "since": since, + } + + var resp response + err := client.GraphQL(query, variables, &resp) + if err != nil { + return nil, err + } + + var assigned []Issue + for _, edge := range resp.Assigned.Issues.Edges { + assigned = append(assigned, edge.Node) + } + + var mentioned []Issue + for _, edge := range resp.Mentioned.Issues.Edges { + mentioned = append(mentioned, edge.Node) + } + + var recent []Issue + for _, edge := range resp.Recent.Issues.Edges { + recent = append(recent, edge.Node) + } + + payload := IssuesPayload{ + assigned, + mentioned, + recent, + } + + return &payload, nil +} + +func IssueList(client *Client, ghRepo Repo, state string) ([]Issue, error) { + var states []string + switch state { + case "open", "": + states = []string{"OPEN"} + case "closed": + states = []string{"CLOSED"} + case "all": + states = []string{"OPEN", "CLOSED"} + default: + return nil, fmt.Errorf("invalid state: %s", state) + } + + query := ` + fragment issue on Issue { + number + title + } + query($owner: String!, $repo: String!, $per_page: Int = 10, $states: [IssueState!] = OPEN) { + repository(owner: $owner, name: $repo) { + issues(first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}, states: $states) { + edges { + node { + ...issue + } + } + } + } + } + ` + + owner := ghRepo.RepoOwner() + repo := ghRepo.RepoName() + variables := map[string]interface{}{ + "owner": owner, + "repo": repo, + "states": states, + } + + var resp struct { + Repository apiIssues + } + + err := client.GraphQL(query, variables, &resp) + if err != nil { + return nil, err + } + + var issues []Issue + for _, edge := range resp.Repository.Issues.Edges { + issues = append(issues, edge.Node) + } + + return issues, nil +} diff --git a/command/issue.go b/command/issue.go index 8d1fcb1d7..ac7ccbf97 100644 --- a/command/issue.go +++ b/command/issue.go @@ -19,7 +19,7 @@ func init() { &cobra.Command{ Use: "status", Short: "Show status of relevant issues", - RunE: issueList, + RunE: issueStatus, }, &cobra.Command{ Use: "view ", @@ -31,6 +31,16 @@ func init() { issueCmd.AddCommand(issueCreateCmd) issueCreateCmd.Flags().StringArrayP("message", "m", nil, "set title and body") issueCreateCmd.Flags().BoolP("web", "w", false, "open the web browser to create an issue") + + issueListCmd := &cobra.Command{ + Use: "list", + Short: "List open issues", + RunE: issueList, + } + issueListCmd.Flags().StringP("assignee", "a", "", "filter by assignee") + issueListCmd.Flags().StringP("label", "l", "", "Filter by assignee") + issueListCmd.Flags().StringP("state", "s", "", "Filter by state") + issueCmd.AddCommand((issueListCmd)) } var issueCmd = &cobra.Command{ @@ -56,12 +66,43 @@ func issueList(cmd *cobra.Command, args []string) error { return err } + state, err := cmd.Flags().GetString("state") + if err != nil { + return err + } + + issues, err := api.IssueList(apiClient, baseRepo, state) + if err != nil { + return err + } + + if len(issues) > 0 { + printIssues(issues...) + } else { + message := fmt.Sprintf("There are no open issues") + printMessage(message) + } + return nil +} + +func issueStatus(cmd *cobra.Command, args []string) error { + ctx := contextForCommand(cmd) + apiClient, err := apiClientForContext(ctx) + if err != nil { + return err + } + + baseRepo, err := ctx.BaseRepo() + if err != nil { + return err + } + currentUser, err := ctx.AuthLogin() if err != nil { return err } - issuePayload, err := api.Issues(apiClient, baseRepo, currentUser) + issuePayload, err := api.IssueStatus(apiClient, baseRepo, currentUser) if err != nil { return err } diff --git a/command/issue_test.go b/command/issue_test.go index 29d494663..4acc76f6e 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -40,6 +40,32 @@ func TestIssueStatus(t *testing.T) { } } +func TestIssueList(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + + jsonFile, _ := os.Open("../test/fixtures/issueList.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + output, err := test.RunCommand(RootCmd, "issue list") + if err != nil { + t.Errorf("error running command `issue list`: %v", err) + } + + expectedIssues := []*regexp.Regexp{ + regexp.MustCompile(`#1.*won`), + regexp.MustCompile(`#2.*too`), + regexp.MustCompile(`#4.*fore`), + } + + for _, r := range expectedIssues { + if !r.MatchString(output) { + t.Errorf("output did not match regexp /%s/", r) + } + } +} + func TestIssueView(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() diff --git a/test/fixtures/issueList.json b/test/fixtures/issueList.json index 784c0cc8f..b249f1103 100644 --- a/test/fixtures/issueList.json +++ b/test/fixtures/issueList.json @@ -1,47 +1,27 @@ { "data": { - "assigned": { - "issues": { - "edges": [ - { - "node": { - "number": 9, - "title": "corey thinks squash tastes bad" - } - }, - { - "node": { - "number": 10, - "title": "broccoli is a superfood" - } + "issues": { + "edges": [ + { + "node": { + "number": 1, + "title": "number won" } - ] - } - }, - "mentioned": { - "issues": { - "edges": [ - { - "node": { - "number": 8, - "title": "rabbits eat carrots" - } - }, - { - "node": { - "number": 11, - "title": "swiss chard is neutral" - } + }, + { + "node": { + "number": 2, + "title": "number too" } - ] - } + }, + { + "node": { + "number": 4, + "title": "number fore" + } + } + ] }, - "recent": { - "issues": { - "edges": [] - } - }, - "pageInfo": { "hasNextPage": false } } } From 57afc2e69fd8fcbd32592f4e145f3fe20d3a8e69 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 13 Nov 2019 13:55:24 -0800 Subject: [PATCH 03/28] Add assignee --- api/queries_issue.go | 27 +++++++++++++++++++++------ command/issue.go | 18 ++++++++++++++---- 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index 96d0e1b2b..0437c7ec7 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -147,7 +147,7 @@ func IssueStatus(client *Client, ghRepo Repo, currentUsername string) (*IssuesPa return &payload, nil } -func IssueList(client *Client, ghRepo Repo, state string) ([]Issue, error) { +func IssueList(client *Client, ghRepo Repo, state string, labels []string, assigneeString string) ([]Issue, error) { var states []string switch state { case "open", "": @@ -160,14 +160,27 @@ func IssueList(client *Client, ghRepo Repo, state string) ([]Issue, error) { return nil, fmt.Errorf("invalid state: %s", state) } + // If you don't want to filter by lables, graphql requires you need + // to send nil instead of an empty array. + if len(labels) == 0 { + labels = nil + } + + var assignee interface{} + if len(assigneeString) > 0 { + assignee = assigneeString + } else { + assignee = nil + } + query := ` fragment issue on Issue { number title } - query($owner: String!, $repo: String!, $per_page: Int = 10, $states: [IssueState!] = OPEN) { + query($owner: String!, $repo: String!, $per_page: Int = 10, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String) { repository(owner: $owner, name: $repo) { - issues(first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}, states: $states) { + issues(first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, labels: $labels, filterBy: {assignee: $assignee}) { edges { node { ...issue @@ -181,9 +194,11 @@ func IssueList(client *Client, ghRepo Repo, state string) ([]Issue, error) { owner := ghRepo.RepoOwner() repo := ghRepo.RepoName() variables := map[string]interface{}{ - "owner": owner, - "repo": repo, - "states": states, + "owner": owner, + "repo": repo, + "states": states, + "labels": labels, + "assignee": assignee, } var resp struct { diff --git a/command/issue.go b/command/issue.go index ac7ccbf97..acce47dd7 100644 --- a/command/issue.go +++ b/command/issue.go @@ -37,9 +37,9 @@ func init() { Short: "List open issues", RunE: issueList, } - issueListCmd.Flags().StringP("assignee", "a", "", "filter by assignee") - issueListCmd.Flags().StringP("label", "l", "", "Filter by assignee") - issueListCmd.Flags().StringP("state", "s", "", "Filter by state") + issueListCmd.Flags().StringP("assignee", "a", "", "Filter by assignee") + issueListCmd.Flags().StringSliceP("label", "l", nil, "Filter by labels ") + issueListCmd.Flags().StringP("state", "s", "", "Filter by state (open, closed or all)") issueCmd.AddCommand((issueListCmd)) } @@ -71,7 +71,17 @@ func issueList(cmd *cobra.Command, args []string) error { return err } - issues, err := api.IssueList(apiClient, baseRepo, state) + labels, err := cmd.Flags().GetStringSlice("label") + if err != nil { + return err + } + + assignee, err := cmd.Flags().GetString("assignee") + if err != nil { + return err + } + + issues, err := api.IssueList(apiClient, baseRepo, state, labels, assignee) if err != nil { return err } From c7d275c520694b01c2e3173cd5f55808f9962d2e Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 13 Nov 2019 13:57:48 -0800 Subject: [PATCH 04/28] Includ body in debug output --- api/client.go | 1 + 1 file changed, 1 insertion(+) diff --git a/api/client.go b/api/client.go index a3fa8e72a..3eac38393 100644 --- a/api/client.go +++ b/api/client.go @@ -38,6 +38,7 @@ func VerboseLog(out io.Writer) ClientOption { return func(tr http.RoundTripper) http.RoundTripper { return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { fmt.Fprintf(out, "> %s %s\n", req.Method, req.URL.RequestURI()) + fmt.Fprintf(out, "> BODY %s\n", req.Body) res, err := tr.RoundTrip(req) if err == nil { fmt.Fprintf(out, "< HTTP %s\n", res.Status) From dc0de147c914da9fb88d756451f6ea119faeaa99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 14 Nov 2019 19:28:13 +0100 Subject: [PATCH 05/28] Add `pr checkout` command --- api/queries.go | 57 +++++++++++++- command/pr.go | 106 +++++++++++++++++++++++++ command/pr_checkout_test.go | 153 ++++++++++++++++++++++++++++++++++++ context/remote.go | 10 +++ 4 files changed, 325 insertions(+), 1 deletion(-) create mode 100644 command/pr_checkout_test.go diff --git a/api/queries.go b/api/queries.go index 5560aa268..7bd121441 100644 --- a/api/queries.go +++ b/api/queries.go @@ -17,7 +17,20 @@ type PullRequest struct { State string URL string HeadRefName string - Reviews struct { + + HeadRepositoryOwner struct { + Login string + } + HeadRepository struct { + Name string + DefaultBranchRef struct { + Name string + } + } + IsCrossRepository bool + MaintainerCanModify bool + + Reviews struct { Nodes []struct { State string Author struct { @@ -355,6 +368,48 @@ func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername st return &payload, nil } +func PullRequestByNumber(client *Client, ghRepo Repo, number int) (*PullRequest, error) { + type response struct { + Repository struct { + PullRequest PullRequest + } + } + + query := ` + query($owner: String!, $repo: String!, $pr_number: Int!) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $pr_number) { + headRefName + headRepositoryOwner { + login + } + headRepository { + name + defaultBranchRef { + name + } + } + isCrossRepository + maintainerCanModify + } + } + }` + + variables := map[string]interface{}{ + "owner": ghRepo.RepoOwner(), + "repo": ghRepo.RepoName(), + "pr_number": number, + } + + var resp response + err := client.GraphQL(query, variables, &resp) + if err != nil { + return nil, err + } + + return &resp.Repository.PullRequest, nil +} + func PullRequestsForBranch(client *Client, ghRepo Repo, branch string) ([]PullRequest, error) { type response struct { Repository struct { diff --git a/command/pr.go b/command/pr.go index a70eaab73..aa774a2d3 100644 --- a/command/pr.go +++ b/command/pr.go @@ -3,9 +3,11 @@ package command import ( "fmt" "os" + "os/exec" "strconv" "github.com/github/gh-cli/api" + "github.com/github/gh-cli/git" "github.com/github/gh-cli/utils" "github.com/spf13/cobra" "golang.org/x/crypto/ssh/terminal" @@ -13,6 +15,7 @@ import ( func init() { RootCmd.AddCommand(prCmd) + prCmd.AddCommand(prCheckoutCmd) prCmd.AddCommand(prCreateCmd) prCmd.AddCommand(prListCmd) prCmd.AddCommand(prStatusCmd) @@ -29,6 +32,12 @@ var prCmd = &cobra.Command{ Short: "Work with pull requests", Long: `Helps you work with pull requests.`, } +var prCheckoutCmd = &cobra.Command{ + Use: "checkout ", + Short: "check out a pull request in git", + Args: cobra.MinimumNArgs(1), + RunE: prCheckout, +} var prListCmd = &cobra.Command{ Use: "list", Short: "List pull requests", @@ -247,6 +256,103 @@ func prView(cmd *cobra.Command, args []string) error { return utils.OpenInBrowser(openURL) } +func prCheckout(cmd *cobra.Command, args []string) error { + prNumber, err := strconv.Atoi(args[0]) + if err != nil { + return err + } + + ctx := contextForCommand(cmd) + currentBranch, err := ctx.Branch() + if err != nil { + return err + } + 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 := api.PullRequestByNumber(apiClient, baseRemote, prNumber) + 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.HasFile("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", prNumber) + 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(prs ...api.PullRequest) { for _, pr := range prs { prNumber := fmt.Sprintf("#%d", pr.Number) diff --git a/command/pr_checkout_test.go b/command/pr_checkout_test.go new file mode 100644 index 000000000..99dc2281d --- /dev/null +++ b/command/pr_checkout_test.go @@ -0,0 +1,153 @@ +package command + +import ( + "bytes" + "os/exec" + "strings" + "testing" + + "github.com/github/gh-cli/context" + "github.com/github/gh-cli/utils" +) + +func TestPRCheckout_sameRepo(t *testing.T) { + 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": { "repository": { "pullRequest": { + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": false, + "maintainerCanModify": false + } } } } + `)) + + ranCommands := [][]string{} + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + ranCommands = append(ranCommands, cmd.Args) + return &outputStub{} + }) + defer restoreCmd() + + RootCmd.SetArgs([]string{"pr", "checkout", "123"}) + _, err := prCheckoutCmd.ExecuteC() + eq(t, err, nil) + + eq(t, len(ranCommands), 6) + eq(t, strings.Join(ranCommands[0], " "), "git rev-parse -q --git-path refs/heads/feature") + eq(t, strings.Join(ranCommands[1], " "), "git rev-parse -q --git-dir") + eq(t, strings.Join(ranCommands[2], " "), "git fetch origin +refs/heads/feature:refs/remotes/origin/feature") + eq(t, strings.Join(ranCommands[3], " "), "git checkout -b feature --no-track origin/feature") + eq(t, strings.Join(ranCommands[4], " "), "git config branch.feature.remote origin") + eq(t, strings.Join(ranCommands[5], " "), "git config branch.feature.merge refs/heads/feature") +} + +func TestPRCheckout_differentRepo(t *testing.T) { + 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": { "repository": { "pullRequest": { + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": true, + "maintainerCanModify": false + } } } } + `)) + + ranCommands := [][]string{} + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + ranCommands = append(ranCommands, cmd.Args) + return &outputStub{} + }) + defer restoreCmd() + + RootCmd.SetArgs([]string{"pr", "checkout", "123"}) + _, err := prCheckoutCmd.ExecuteC() + eq(t, err, nil) + + eq(t, len(ranCommands), 5) + eq(t, strings.Join(ranCommands[0], " "), "git config branch.feature.merge") + eq(t, strings.Join(ranCommands[1], " "), "git fetch origin refs/pull/123/head:feature") + eq(t, strings.Join(ranCommands[2], " "), "git checkout feature") + eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.remote origin") + eq(t, strings.Join(ranCommands[4], " "), "git config branch.feature.merge refs/pull/123/head") +} + +func TestPRCheckout_maintainerCanModify(t *testing.T) { + 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": { "repository": { "pullRequest": { + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": true, + "maintainerCanModify": true + } } } } + `)) + + ranCommands := [][]string{} + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + ranCommands = append(ranCommands, cmd.Args) + return &outputStub{} + }) + defer restoreCmd() + + RootCmd.SetArgs([]string{"pr", "checkout", "123"}) + _, err := prCheckoutCmd.ExecuteC() + eq(t, err, nil) + + eq(t, len(ranCommands), 5) + eq(t, strings.Join(ranCommands[0], " "), "git config branch.feature.merge") + eq(t, strings.Join(ranCommands[1], " "), "git fetch origin refs/pull/123/head:feature") + eq(t, strings.Join(ranCommands[2], " "), "git checkout feature") + eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.remote https://github.com/hubot/REPO.git") + eq(t, strings.Join(ranCommands[4], " "), "git config branch.feature.merge refs/heads/feature") +} diff --git a/context/remote.go b/context/remote.go index 9f3b228dc..a30ac6c45 100644 --- a/context/remote.go +++ b/context/remote.go @@ -25,6 +25,16 @@ func (r Remotes) FindByName(names ...string) (*Remote, error) { return nil, fmt.Errorf("no GitHub remotes found") } +// FindByRepo returns the first Remote that points to a specific GitHub repository +func (r Remotes) FindByRepo(owner, name string) (*Remote, error) { + for _, rem := range r { + if strings.EqualFold(rem.Owner, owner) && strings.EqualFold(rem.Name, name) { + return rem, nil + } + } + return nil, fmt.Errorf("no matching remote found") +} + // Remote represents a git remote mapped to a GitHub repository type Remote struct { *git.Remote From 2616da04c469c81e834eb13f4fae926555718036 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 14 Nov 2019 20:02:42 +0100 Subject: [PATCH 06/28] Only capture stderr of commands that don't have configured Stderr --- git/git.go | 1 - utils/prepare_cmd.go | 6 ++++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/git/git.go b/git/git.go index 411863b31..318a9ddcc 100644 --- a/git/git.go +++ b/git/git.go @@ -33,7 +33,6 @@ func Dir() (string, error) { func WorkdirName() (string, error) { toplevelCmd := exec.Command("git", "rev-parse", "--show-toplevel") - toplevelCmd.Stderr = nil output, err := utils.PrepareCmd(toplevelCmd).Output() dir := firstLine(output) if dir == "" { diff --git a/utils/prepare_cmd.go b/utils/prepare_cmd.go index 0129d1a5b..6b354cb80 100644 --- a/utils/prepare_cmd.go +++ b/utils/prepare_cmd.go @@ -38,6 +38,9 @@ func (c cmdWithStderr) Output() ([]byte, error) { if os.Getenv("DEBUG") != "" { fmt.Fprintf(os.Stderr, "%v\n", c.Cmd.Args) } + if c.Cmd.Stderr != nil { + return c.Cmd.Output() + } errStream := &bytes.Buffer{} c.Cmd.Stderr = errStream out, err := c.Cmd.Output() @@ -51,6 +54,9 @@ func (c cmdWithStderr) Run() error { if os.Getenv("DEBUG") != "" { fmt.Fprintf(os.Stderr, "%v\n", c.Cmd.Args) } + if c.Cmd.Stderr != nil { + return c.Cmd.Run() + } errStream := &bytes.Buffer{} c.Cmd.Stderr = errStream err := c.Cmd.Run() From 1fc9c8f9d63edb9455244e4679159a9ff9904bba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 14 Nov 2019 20:29:24 +0100 Subject: [PATCH 07/28] Test the scenario where the target branch already exists --- command/pr.go | 2 +- command/pr_checkout_test.go | 71 ++++++++++++++++++++++++++++++++----- command/testing.go | 14 ++++++++ git/git.go | 29 +++------------ 4 files changed, 81 insertions(+), 35 deletions(-) diff --git a/command/pr.go b/command/pr.go index aa774a2d3..2f59934a7 100644 --- a/command/pr.go +++ b/command/pr.go @@ -302,7 +302,7 @@ func prCheckout(cmd *cobra.Command, args []string) error { cmdQueue = append(cmdQueue, []string{"git", "fetch", headRemote.Name, refSpec}) // local branch already exists - if git.HasFile("refs", "heads", newBranchName) { + 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 { diff --git a/command/pr_checkout_test.go b/command/pr_checkout_test.go index 99dc2281d..9f85f4f95 100644 --- a/command/pr_checkout_test.go +++ b/command/pr_checkout_test.go @@ -40,8 +40,13 @@ func TestPRCheckout_sameRepo(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { - ranCommands = append(ranCommands, cmd.Args) - return &outputStub{} + switch strings.Join(cmd.Args, " ") { + case "git show-ref --verify --quiet refs/heads/feature": + return &errorStub{"exit status: 1"} + default: + ranCommands = append(ranCommands, cmd.Args) + return &outputStub{} + } }) defer restoreCmd() @@ -49,13 +54,61 @@ func TestPRCheckout_sameRepo(t *testing.T) { _, err := prCheckoutCmd.ExecuteC() eq(t, err, nil) - eq(t, len(ranCommands), 6) - eq(t, strings.Join(ranCommands[0], " "), "git rev-parse -q --git-path refs/heads/feature") - eq(t, strings.Join(ranCommands[1], " "), "git rev-parse -q --git-dir") - eq(t, strings.Join(ranCommands[2], " "), "git fetch origin +refs/heads/feature:refs/remotes/origin/feature") - eq(t, strings.Join(ranCommands[3], " "), "git checkout -b feature --no-track origin/feature") - eq(t, strings.Join(ranCommands[4], " "), "git config branch.feature.remote origin") - eq(t, strings.Join(ranCommands[5], " "), "git config branch.feature.merge refs/heads/feature") + 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") +} + +func TestPRCheckout_existingBranch(t *testing.T) { + 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": { "repository": { "pullRequest": { + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": false, + "maintainerCanModify": false + } } } } + `)) + + 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": + return &outputStub{} + default: + ranCommands = append(ranCommands, cmd.Args) + return &outputStub{} + } + }) + defer restoreCmd() + + RootCmd.SetArgs([]string{"pr", "checkout", "123"}) + _, err := prCheckoutCmd.ExecuteC() + eq(t, err, nil) + + 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[2], " "), "git merge --ff-only refs/remotes/origin/feature") } func TestPRCheckout_differentRepo(t *testing.T) { diff --git a/command/testing.go b/command/testing.go index 2e1cf9505..7758d4a8b 100644 --- a/command/testing.go +++ b/command/testing.go @@ -1,6 +1,8 @@ package command import ( + "errors" + "github.com/github/gh-cli/api" "github.com/github/gh-cli/context" ) @@ -34,3 +36,15 @@ func (s outputStub) Output() ([]byte, error) { func (s outputStub) Run() error { return nil } + +type errorStub struct { + message string +} + +func (s errorStub) Output() ([]byte, error) { + return nil, errors.New(s.message) +} + +func (s errorStub) Run() error { + return errors.New(s.message) +} diff --git a/git/git.go b/git/git.go index 318a9ddcc..8824872a4 100644 --- a/git/git.go +++ b/git/git.go @@ -41,31 +41,10 @@ func WorkdirName() (string, error) { return dir, err } -func HasFile(segments ...string) bool { - // The blessed way to resolve paths within git dir since Git 2.5.0 - pathCmd := exec.Command("git", "rev-parse", "-q", "--git-path", filepath.Join(segments...)) - if output, err := utils.PrepareCmd(pathCmd).Output(); err == nil { - if lines := outputLines(output); len(lines) == 1 { - if _, err := os.Stat(lines[0]); err == nil { - return true - } - } - } - - // Fallback for older git versions - dir, err := Dir() - if err != nil { - return false - } - - s := []string{dir} - s = append(s, segments...) - path := filepath.Join(s...) - if _, err := os.Stat(path); err == nil { - return true - } - - return false +func VerifyRef(ref string) bool { + showRef := exec.Command("git", "show-ref", "--verify", "--quiet", ref) + err := utils.PrepareCmd(showRef).Run() + return err == nil } func BranchAtRef(paths ...string) (name string, err error) { From b4b0c37febf0904a73e4fb89130ab82ac986d15f Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Thu, 14 Nov 2019 11:30:53 -0800 Subject: [PATCH 08/28] show labels --- api/queries_issue.go | 86 +++++++++++++++++++++++++++++--------------- command/issue.go | 25 ++++++++++--- go.mod | 1 + go.sum | 2 ++ 4 files changed, 81 insertions(+), 33 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index 0437c7ec7..bbf9b276c 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -51,15 +51,29 @@ type IssuesPayload struct { } type Issue struct { - Number int - Title string - URL string + Number int + Title string + URL string + Labels []string + TotalLabelCount int } type apiIssues struct { Issues struct { Edges []struct { - Node Issue + Node struct { + Number int + Title string + URL string + Labels struct { + Edges []struct { + Node struct { + Name string + } + } + TotalCount int + } + } } } } @@ -123,20 +137,9 @@ func IssueStatus(client *Client, ghRepo Repo, currentUsername string) (*IssuesPa return nil, err } - var assigned []Issue - for _, edge := range resp.Assigned.Issues.Edges { - assigned = append(assigned, edge.Node) - } - - var mentioned []Issue - for _, edge := range resp.Mentioned.Issues.Edges { - mentioned = append(mentioned, edge.Node) - } - - var recent []Issue - for _, edge := range resp.Recent.Issues.Edges { - recent = append(recent, edge.Node) - } + assigned := convertAPIToIssues(resp.Assigned) + mentioned := convertAPIToIssues(resp.Mentioned) + recent := convertAPIToIssues(resp.Recent) payload := IssuesPayload{ assigned, @@ -147,7 +150,7 @@ func IssueStatus(client *Client, ghRepo Repo, currentUsername string) (*IssuesPa return &payload, nil } -func IssueList(client *Client, ghRepo Repo, state string, labels []string, assigneeString string) ([]Issue, error) { +func IssueList(client *Client, ghRepo Repo, state string, labels []string, assigneeString string, limit int) ([]Issue, error) { var states []string switch state { case "open", "": @@ -176,11 +179,20 @@ func IssueList(client *Client, ghRepo Repo, state string, labels []string, assig query := ` fragment issue on Issue { number - title - } - query($owner: String!, $repo: String!, $per_page: Int = 10, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String) { + title + labels(first: 3) { + edges { + node { + name + } + } + totalCount + } + } + + query($owner: String!, $repo: String!, $limit: Int, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String) { repository(owner: $owner, name: $repo) { - issues(first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, labels: $labels, filterBy: {assignee: $assignee}) { + issues(first: $limit, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, labels: $labels, filterBy: {assignee: $assignee}) { edges { node { ...issue @@ -194,6 +206,7 @@ func IssueList(client *Client, ghRepo Repo, state string, labels []string, assig owner := ghRepo.RepoOwner() repo := ghRepo.RepoName() variables := map[string]interface{}{ + "limit": limit, "owner": owner, "repo": repo, "states": states, @@ -210,10 +223,27 @@ func IssueList(client *Client, ghRepo Repo, state string, labels []string, assig return nil, err } - var issues []Issue - for _, edge := range resp.Repository.Issues.Edges { - issues = append(issues, edge.Node) - } - + issues := convertAPIToIssues(resp.Repository) return issues, nil } + +func convertAPIToIssues(i apiIssues) []Issue { + var issues []Issue + for _, edge := range i.Issues.Edges { + var labels []string + for _, labelEdge := range edge.Node.Labels.Edges { + labels = append(labels, labelEdge.Node.Name) + } + + issue := Issue{ + Number: edge.Node.Number, + Title: edge.Node.Title, + URL: edge.Node.URL, + Labels: labels, + TotalLabelCount: edge.Node.Labels.TotalCount, + } + issues = append(issues, issue) + } + + return issues +} diff --git a/command/issue.go b/command/issue.go index acce47dd7..22fc897b8 100644 --- a/command/issue.go +++ b/command/issue.go @@ -37,9 +37,10 @@ func init() { Short: "List open issues", RunE: issueList, } - issueListCmd.Flags().StringP("assignee", "a", "", "Filter by assignee") - issueListCmd.Flags().StringSliceP("label", "l", nil, "Filter by labels ") - issueListCmd.Flags().StringP("state", "s", "", "Filter by state (open, closed or all)") + 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 items to fetch (default ") issueCmd.AddCommand((issueListCmd)) } @@ -81,7 +82,12 @@ func issueList(cmd *cobra.Command, args []string) error { return err } - issues, err := api.IssueList(apiClient, baseRepo, state, labels, assignee) + limit, err := cmd.Flags().GetInt("limit") + if err != nil { + return err + } + + issues, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit) if err != nil { return err } @@ -238,6 +244,15 @@ func issueCreate(cmd *cobra.Command, args []string) error { func printIssues(issues ...api.Issue) { for _, issue := range issues { - fmt.Printf(" #%d %s\n", issue.Number, truncate(70, issue.Title)) + number := utils.Green("#" + strconv.Itoa(issue.Number)) + var coloredLabels string + if len(issue.Labels) > 0 { + var ellipse string + if issue.TotalLabelCount > len(issue.Labels) { + ellipse = "…" + } + coloredLabels = utils.Gray(fmt.Sprintf(" (%s%s)", strings.Join(issue.Labels, ", "), ellipse)) + } + fmt.Printf(" %s %s %s\n", number, truncate(70, issue.Title), coloredLabels) } } diff --git a/go.mod b/go.mod index 9c89ff7b1..1003bae41 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/github/gh-cli go 1.13 require ( + github.com/fatih/color v1.7.0 github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 github.com/mattn/go-colorable v0.1.2 github.com/mattn/go-isatty v0.0.9 diff --git a/go.sum b/go.sum index af7d46f56..b94183d79 100644 --- a/go.sum +++ b/go.sum @@ -7,6 +7,8 @@ github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3Ee github.com/cpuguy83/go-md2man v1.0.10/go.mod h1:SmD6nW6nTyfqj6ABTjUi3V3JVMnlJmwcJI5acqYI6dE= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/fatih/color v1.7.0 h1:DkWD4oS2D8LGGgTQ6IvwJJXSL5Vp2ffcQg58nFV38Ys= +github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM= From f0e6ec2b5eb0b8d9129a20cf2c580bc7283c7ea9 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Thu, 14 Nov 2019 11:46:31 -0800 Subject: [PATCH 09/28] Nope --- go.mod | 1 - 1 file changed, 1 deletion(-) diff --git a/go.mod b/go.mod index 1003bae41..9c89ff7b1 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,6 @@ module github.com/github/gh-cli go 1.13 require ( - github.com/fatih/color v1.7.0 github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 github.com/mattn/go-colorable v0.1.2 github.com/mattn/go-isatty v0.0.9 From 4fab43a667752c873a9e44318bff5079ac7bef40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 14 Nov 2019 20:53:34 +0100 Subject: [PATCH 10/28] Cover more `pr checkout` test scenarios --- command/pr_checkout_test.go | 190 +++++++++++++++++++++++++++++++++--- context/remote.go | 2 +- 2 files changed, 175 insertions(+), 17 deletions(-) diff --git a/command/pr_checkout_test.go b/command/pr_checkout_test.go index 9f85f4f95..930d2b4d8 100644 --- a/command/pr_checkout_test.go +++ b/command/pr_checkout_test.go @@ -111,6 +111,58 @@ func TestPRCheckout_existingBranch(t *testing.T) { eq(t, strings.Join(ranCommands[2], " "), "git merge --ff-only refs/remotes/origin/feature") } +func TestPRCheckout_differentRepo_remoteExists(t *testing.T) { + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + "robot-fork": "hubot/REPO", + }) + initContext = func() context.Context { + return ctx + } + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequest": { + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": true, + "maintainerCanModify": false + } } } } + `)) + + 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": + return &errorStub{"exit status: 1"} + default: + ranCommands = append(ranCommands, cmd.Args) + return &outputStub{} + } + }) + defer restoreCmd() + + RootCmd.SetArgs([]string{"pr", "checkout", "123"}) + _, err := prCheckoutCmd.ExecuteC() + eq(t, err, nil) + + 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") +} + func TestPRCheckout_differentRepo(t *testing.T) { ctx := context.NewBlank() ctx.SetBranch("master") @@ -141,8 +193,13 @@ func TestPRCheckout_differentRepo(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { - ranCommands = append(ranCommands, cmd.Args) - return &outputStub{} + switch strings.Join(cmd.Args, " ") { + case "git config branch.feature.merge": + return &errorStub{"exit status 1"} + default: + ranCommands = append(ranCommands, cmd.Args) + return &outputStub{} + } }) defer restoreCmd() @@ -150,12 +207,109 @@ func TestPRCheckout_differentRepo(t *testing.T) { _, err := prCheckoutCmd.ExecuteC() eq(t, err, nil) - eq(t, len(ranCommands), 5) - eq(t, strings.Join(ranCommands[0], " "), "git config branch.feature.merge") - eq(t, strings.Join(ranCommands[1], " "), "git fetch origin refs/pull/123/head:feature") - eq(t, strings.Join(ranCommands[2], " "), "git checkout feature") - eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.remote origin") - eq(t, strings.Join(ranCommands[4], " "), "git config branch.feature.merge refs/pull/123/head") + 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") +} + +func TestPRCheckout_differentRepo_existingBranch(t *testing.T) { + 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": { "repository": { "pullRequest": { + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": true, + "maintainerCanModify": false + } } } } + `)) + + ranCommands := [][]string{} + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + switch strings.Join(cmd.Args, " ") { + case "git config branch.feature.merge": + return &outputStub{[]byte("refs/heads/feature\n")} + default: + ranCommands = append(ranCommands, cmd.Args) + return &outputStub{} + } + }) + defer restoreCmd() + + RootCmd.SetArgs([]string{"pr", "checkout", "123"}) + _, err := prCheckoutCmd.ExecuteC() + eq(t, err, nil) + + 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") +} + +func TestPRCheckout_differentRepo_currentBranch(t *testing.T) { + ctx := context.NewBlank() + ctx.SetBranch("feature") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequest": { + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": true, + "maintainerCanModify": false + } } } } + `)) + + ranCommands := [][]string{} + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + switch strings.Join(cmd.Args, " ") { + case "git config branch.feature.merge": + return &outputStub{[]byte("refs/heads/feature\n")} + default: + ranCommands = append(ranCommands, cmd.Args) + return &outputStub{} + } + }) + defer restoreCmd() + + RootCmd.SetArgs([]string{"pr", "checkout", "123"}) + _, err := prCheckoutCmd.ExecuteC() + eq(t, err, nil) + + eq(t, len(ranCommands), 2) + eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head") + eq(t, strings.Join(ranCommands[1], " "), "git merge --ff-only FETCH_HEAD") } func TestPRCheckout_maintainerCanModify(t *testing.T) { @@ -188,8 +342,13 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) { ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { - ranCommands = append(ranCommands, cmd.Args) - return &outputStub{} + switch strings.Join(cmd.Args, " ") { + case "git config branch.feature.merge": + return &errorStub{"exit status 1"} + default: + ranCommands = append(ranCommands, cmd.Args) + return &outputStub{} + } }) defer restoreCmd() @@ -197,10 +356,9 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) { _, err := prCheckoutCmd.ExecuteC() eq(t, err, nil) - eq(t, len(ranCommands), 5) - eq(t, strings.Join(ranCommands[0], " "), "git config branch.feature.merge") - eq(t, strings.Join(ranCommands[1], " "), "git fetch origin refs/pull/123/head:feature") - eq(t, strings.Join(ranCommands[2], " "), "git checkout feature") - eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.remote https://github.com/hubot/REPO.git") - eq(t, strings.Join(ranCommands[4], " "), "git config branch.feature.merge refs/heads/feature") + 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") } diff --git a/context/remote.go b/context/remote.go index a30ac6c45..6c41eeaf1 100644 --- a/context/remote.go +++ b/context/remote.go @@ -28,7 +28,7 @@ func (r Remotes) FindByName(names ...string) (*Remote, error) { // FindByRepo returns the first Remote that points to a specific GitHub repository func (r Remotes) FindByRepo(owner, name string) (*Remote, error) { for _, rem := range r { - if strings.EqualFold(rem.Owner, owner) && strings.EqualFold(rem.Name, name) { + if strings.EqualFold(rem.RepoOwner(), owner) && strings.EqualFold(rem.RepoName(), name) { return rem, nil } } From d8308b4c15e476a9473d091a9447fb922e620f24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 14 Nov 2019 20:54:44 +0100 Subject: [PATCH 11/28] Allow `pr checkout` while not on any branch --- command/pr.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/command/pr.go b/command/pr.go index 2f59934a7..6ffa56e8e 100644 --- a/command/pr.go +++ b/command/pr.go @@ -263,10 +263,7 @@ func prCheckout(cmd *cobra.Command, args []string) error { } ctx := contextForCommand(cmd) - currentBranch, err := ctx.Branch() - if err != nil { - return err - } + currentBranch, _ := ctx.Branch() remotes, err := ctx.Remotes() if err != nil { return err From 761d29cf5e2089fda0f7e0aa880686818f39e553 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Thu, 14 Nov 2019 12:40:48 -0800 Subject: [PATCH 12/28] Fix test --- command/issue_test.go | 6 ++- test/fixtures/issueList.json | 78 ++++++++++++++++++++++++++---------- 2 files changed, 60 insertions(+), 24 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index 4acc76f6e..be93304b6 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -35,7 +35,8 @@ func TestIssueStatus(t *testing.T) { for _, r := range expectedIssues { if !r.MatchString(output) { - t.Errorf("output did not match regexp /%s/", r) + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) + return } } } @@ -61,7 +62,8 @@ func TestIssueList(t *testing.T) { for _, r := range expectedIssues { if !r.MatchString(output) { - t.Errorf("output did not match regexp /%s/", r) + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) + return } } } diff --git a/test/fixtures/issueList.json b/test/fixtures/issueList.json index b249f1103..17e92c743 100644 --- a/test/fixtures/issueList.json +++ b/test/fixtures/issueList.json @@ -1,27 +1,61 @@ { "data": { - "issues": { - "edges": [ - { - "node": { - "number": 1, - "title": "number won" + "repository": { + "issues": { + "edges": [ + { + "node": { + "number": 1, + "title": "number won", + "url": "https://wow.com", + "labels": { + "edges": [ + { + "node": { + "name": "label" + } + } + ], + "totalCount": 1 + } + } + }, + { + "node": { + "number": 2, + "title": "number too", + "url": "https://wow.com", + "labels": { + "edges": [ + { + "node": { + "name": "label" + } + } + ], + "totalCount": 1 + } + } + }, + { + "node": { + "number": 4, + "title": "number fore", + "url": "https://wow.com", + "labels": { + "edges": [ + { + "node": { + "name": "label" + } + } + ], + "totalCount": 1 + } + } } - }, - { - "node": { - "number": 2, - "title": "number too" - } - }, - { - "node": { - "number": 4, - "title": "number fore" - } - } - ] - }, - "pageInfo": { "hasNextPage": false } + ] + } + } } } From 04f20ddb2bf6b33abd4fe468abec0e02d99d40d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 15 Nov 2019 11:43:00 +0100 Subject: [PATCH 13/28] Use `statusCheckRollup` internal GraphQL to simplify Statuses+Checks With the old approach, we had to enumerate all StatusContexts and CheckRuns to calculate whether a PR has passing or failing CI status. Using `statusCheckRollup` which is behind the `pe_mobile` feature flag, this is somewhat simpler because both StatusContexts and CheckRuns are now behind the same connection. Additionally, should we decide to *not* show the number of passing/failing checks, this now approach allows us to consume the `statusCheckRollup { state }` field and avoid paginated collections and calculating the "winning" state altogether. --- api/queries.go | 61 ++++++++++++++++++------------------------------- command/root.go | 1 + 2 files changed, 23 insertions(+), 39 deletions(-) diff --git a/api/queries.go b/api/queries.go index 4f1f74ed5..d18b2c679 100644 --- a/api/queries.go +++ b/api/queries.go @@ -35,17 +35,11 @@ type PullRequest struct { Commits struct { Nodes []struct { Commit struct { - Status struct { - Contexts []struct { - State string - } - } - CheckSuites struct { - Nodes []struct { - CheckRuns struct { - Nodes []struct { - Conclusion string - } + StatusCheckRollup struct { + Contexts struct { + Nodes []struct { + State string + Conclusion string } } } @@ -97,32 +91,23 @@ func (pr *PullRequest) ChecksStatus() (summary PullRequestChecksStatus) { return } commit := pr.Commits.Nodes[0].Commit - for _, status := range commit.Status.Contexts { - switch status.State { - case "SUCCESS": + for _, c := range commit.StatusCheckRollup.Contexts.Nodes { + state := c.State + if state == "" { + state = c.Conclusion + } + switch state { + case "SUCCESS", "NEUTRAL", "SKIPPED": summary.Passing++ - case "EXPECTED", "ERROR", "FAILURE": + case "ERROR", "FAILURE", "CANCELLED", "TIMED_OUT", "ACTION_REQUIRED": summary.Failing++ - case "PENDING": + case "EXPECTED", "QUEUED", "PENDING", "IN_PROGRESS": summary.Pending++ default: - panic(fmt.Errorf("unsupported status: %q", status.State)) + panic(fmt.Errorf("unsupported status: %q", state)) } summary.Total++ } - for _, checkSuite := range commit.CheckSuites.Nodes { - for _, checkRun := range checkSuite.CheckRuns.Nodes { - switch checkRun.Conclusion { - case "SUCCESS", "NEUTRAL": - summary.Passing++ - case "FAILURE", "CANCELLED", "TIMED_OUT", "ACTION_REQUIRED": - summary.Failing++ - default: - panic(fmt.Errorf("unsupported check conclusion: %q", checkRun.Conclusion)) - } - summary.Total++ - } - } return } @@ -267,15 +252,13 @@ func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername st commits(last: 1) { nodes { commit { - status { - contexts { - state - } - } - checkSuites(first: 50) { - nodes { - checkRuns(first: 50) { - nodes { + statusCheckRollup { + contexts(last: 100) { + nodes { + ...on StatusContext { + state + } + ...on CheckRun { conclusion } } diff --git a/command/root.go b/command/root.go index 22c0122a1..4fe921391 100644 --- a/command/root.go +++ b/command/root.go @@ -75,6 +75,7 @@ var apiClientForContext = func(ctx context.Context) (*api.Client, error) { // antiope-preview: Checks // shadow-cat-preview: Draft pull requests api.AddHeader("Accept", "application/vnd.github.antiope-preview+json, application/vnd.github.shadow-cat-preview"), + api.AddHeader("GraphQL-Features", "pe_mobile"), } if verbose := os.Getenv("DEBUG"); verbose != "" { opts = append(opts, api.VerboseLog(os.Stderr)) From 6e894a0eabec0f683e2794b961374b8b7f3c65d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 15 Nov 2019 12:00:13 +0100 Subject: [PATCH 14/28] Use `reviewDecision` internal GraphQL to simplify review state Before, we've used the `reviews` connection to iterate through all reviews chronologically and try to guess the final state of reviews. This approach had several problems: - it didn't handle dismissed reviews well, - the conclusion would likely be wrong if the number of total reviews exceeded the per-page limit. The `pe_mobile` feature flag exposes the `reviewDecision` field that handles all of this for us. --- api/queries.go | 36 +++++++----------------------------- 1 file changed, 7 insertions(+), 29 deletions(-) diff --git a/api/queries.go b/api/queries.go index d18b2c679..c4bfa8353 100644 --- a/api/queries.go +++ b/api/queries.go @@ -23,14 +23,7 @@ type PullRequest struct { Login string } - Reviews struct { - Nodes []struct { - State string - Author struct { - Login string - } - } - } + ReviewDecision string Commits struct { Nodes []struct { @@ -62,19 +55,11 @@ type PullRequestReviewStatus struct { func (pr *PullRequest) ReviewStatus() PullRequestReviewStatus { status := PullRequestReviewStatus{} - reviewMap := map[string]string{} - // Reviews will include every review on record, including consecutive ones - // from the same actor. Consolidate them into latest state per reviewer. - for _, review := range pr.Reviews.Nodes { - reviewMap[review.Author.Login] = review.State - } - for _, state := range reviewMap { - switch state { - case "CHANGES_REQUESTED": - status.ChangesRequested = true - case "APPROVED": - status.Approved = true - } + switch pr.ReviewDecision { + case "CHANGES_REQUESTED": + status.ChangesRequested = true + case "APPROVED": + status.Approved = true } return status } @@ -270,14 +255,7 @@ func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername st } fragment prWithReviews on PullRequest { ...pr - reviews(last: 20) { - nodes { - state - author { - login - } - } - } + reviewDecision } query($owner: String!, $repo: String!, $headRefName: String!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) { From be55f81e16992df46bd037b1aece6d4e4e483e41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 15 Nov 2019 12:05:05 +0100 Subject: [PATCH 15/28] Add "review required" notice to PRs if applicable --- api/queries.go | 3 +++ command/pr.go | 2 ++ 2 files changed, 5 insertions(+) diff --git a/api/queries.go b/api/queries.go index c4bfa8353..2e14fb26b 100644 --- a/api/queries.go +++ b/api/queries.go @@ -51,6 +51,7 @@ func (pr PullRequest) HeadLabel() string { type PullRequestReviewStatus struct { ChangesRequested bool Approved bool + ReviewRequired bool } func (pr *PullRequest) ReviewStatus() PullRequestReviewStatus { @@ -60,6 +61,8 @@ func (pr *PullRequest) ReviewStatus() PullRequestReviewStatus { status.ChangesRequested = true case "APPROVED": status.Approved = true + case "REVIEW_REQUIRED": + status.ReviewRequired = true } return status } diff --git a/command/pr.go b/command/pr.go index 0445b81d6..eba61704d 100644 --- a/command/pr.go +++ b/command/pr.go @@ -275,6 +275,8 @@ func printPrs(prs ...api.PullRequest) { if reviews.ChangesRequested { fmt.Printf(" - %s", utils.Red("changes requested")) + } else if reviews.ReviewRequired { + fmt.Printf(" - %s", utils.Yellow("review required")) } else if reviews.Approved { fmt.Printf(" - %s", utils.Green("approved")) } From 3e062589c5b7b0db9f80b8c29617f0b3d4e13b54 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 15 Nov 2019 13:16:19 -0800 Subject: [PATCH 16/28] Update README.md --- README.md | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 5ada69ed2..47f80bd8b 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,14 @@ The #ce-cli team is working on a publicly available CLI tool to reduce the frict This tool is an endeavor separate from [github/hub](https://github.com/github/hub), which acts as a proxy to `git`, since our aim is to reimagine from scratch the kind of command line interface to GitHub that would serve our users' interests best. +# Installation + +_warning, gh is in a very alpha phase_ + +`brew install github/gh/gh` + +That's it. You are now ready to use `gh` on the command line. 🥳 + # Process - [Demo planning doc](https://docs.google.com/document/d/18ym-_xjFTSXe0-xzgaBn13Su7MEhWfLE5qSNPJV4M0A/edit) @@ -14,6 +22,7 @@ This tool is an endeavor separate from [github/hub](https://github.com/github/hu This can all be done from your local terminal. -1. `git tag -a vYOUR_VERSION_NUMBER -m 'vYOUR_VERSION_NUMBER'` -2. `git push origin vYOUR_VERSION_NUMBER` -3. Go to https://github.com/github/homebrew-gh/releases and look at the release (read https://github.com/github/homebrew-gh for how to install the release via homebrew) \ No newline at end of file +1. `git tag 'vVERSION_NUMBER' # example git tag 'v0.0.1'` +2. `git push origin vVERSION_NUMBER` +3. Wait a few minutes for the build to run and CI to pass. Look at the [actions tab](https://github.com/github/gh-cli/actions) to check the progress. +4. Go to https://github.com/github/homebrew-gh/releases and look at the release From e4903fc7c0829905df258a04d6786f3c0ece6358 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 15 Nov 2019 13:20:13 -0800 Subject: [PATCH 17/28] Less pointless words --- command/issue.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/issue.go b/command/issue.go index 22fc897b8..8a9dd8164 100644 --- a/command/issue.go +++ b/command/issue.go @@ -40,7 +40,7 @@ func init() { 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 items to fetch (default ") + issueListCmd.Flags().IntP("limit", "L", 30, "maximum number of issues to fetch") issueCmd.AddCommand((issueListCmd)) } From 927b172135b171261a7abc5518e0f106f9cf5335 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 15 Nov 2019 13:23:05 -0800 Subject: [PATCH 18/28] Only add space to status issues --- command/issue.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/command/issue.go b/command/issue.go index 8a9dd8164..9dfcd502c 100644 --- a/command/issue.go +++ b/command/issue.go @@ -93,7 +93,7 @@ func issueList(cmd *cobra.Command, args []string) error { } if len(issues) > 0 { - printIssues(issues...) + printIssues("", issues...) } else { message := fmt.Sprintf("There are no open issues") printMessage(message) @@ -125,7 +125,7 @@ func issueStatus(cmd *cobra.Command, args []string) error { printHeader("Issues assigned to you") if issuePayload.Assigned != nil { - printIssues(issuePayload.Assigned...) + printIssues(" ", issuePayload.Assigned...) } else { message := fmt.Sprintf(" There are no issues assgined to you") printMessage(message) @@ -134,7 +134,7 @@ func issueStatus(cmd *cobra.Command, args []string) error { printHeader("Issues mentioning you") if len(issuePayload.Mentioned) > 0 { - printIssues(issuePayload.Mentioned...) + printIssues(" ", issuePayload.Mentioned...) } else { printMessage(" There are no issues mentioning you") } @@ -142,7 +142,7 @@ func issueStatus(cmd *cobra.Command, args []string) error { printHeader("Recent issues") if len(issuePayload.Recent) > 0 { - printIssues(issuePayload.Recent...) + printIssues(" ", issuePayload.Recent...) } else { printMessage(" There are no recent issues") } @@ -242,7 +242,7 @@ func issueCreate(cmd *cobra.Command, args []string) error { return nil } -func printIssues(issues ...api.Issue) { +func printIssues(prefix string, issues ...api.Issue) { for _, issue := range issues { number := utils.Green("#" + strconv.Itoa(issue.Number)) var coloredLabels string @@ -253,6 +253,6 @@ func printIssues(issues ...api.Issue) { } coloredLabels = utils.Gray(fmt.Sprintf(" (%s%s)", strings.Join(issue.Labels, ", "), ellipse)) } - fmt.Printf(" %s %s %s\n", number, truncate(70, issue.Title), coloredLabels) + fmt.Printf("%s%s %s %s\n", prefix, number, truncate(70, issue.Title), coloredLabels) } } From a9ce81b139b2c9980c60efa39cf8c85b6fcb91be Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 15 Nov 2019 13:26:27 -0800 Subject: [PATCH 19/28] bye bye --- go.sum | 2 -- 1 file changed, 2 deletions(-) diff --git a/go.sum b/go.sum index b94183d79..af7d46f56 100644 --- a/go.sum +++ b/go.sum @@ -7,8 +7,6 @@ github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3Ee github.com/cpuguy83/go-md2man v1.0.10/go.mod h1:SmD6nW6nTyfqj6ABTjUi3V3JVMnlJmwcJI5acqYI6dE= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/fatih/color v1.7.0 h1:DkWD4oS2D8LGGgTQ6IvwJJXSL5Vp2ffcQg58nFV38Ys= -github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM= From e7fed2e39c44a73275b17938ad3d4346f0c7cbcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 18 Nov 2019 17:12:43 +0100 Subject: [PATCH 20/28] Include open issues only in `issue status` --- api/queries.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/queries.go b/api/queries.go index 4f1f74ed5..e1acf63a7 100644 --- a/api/queries.go +++ b/api/queries.go @@ -165,7 +165,7 @@ func Issues(client *Client, ghRepo Repo, currentUsername string) (*IssuesPayload } query($owner: String!, $repo: String!, $since: DateTime!, $viewer: String!, $per_page: Int = 10) { assigned: repository(owner: $owner, name: $repo) { - issues(filterBy: {assignee: $viewer}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { + issues(filterBy: {assignee: $viewer, states: OPEN}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { edges { node { ...issue @@ -174,7 +174,7 @@ func Issues(client *Client, ghRepo Repo, currentUsername string) (*IssuesPayload } } mentioned: repository(owner: $owner, name: $repo) { - issues(filterBy: {mentioned: $viewer}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { + issues(filterBy: {mentioned: $viewer, states: OPEN}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { edges { node { ...issue @@ -183,7 +183,7 @@ func Issues(client *Client, ghRepo Repo, currentUsername string) (*IssuesPayload } } recent: repository(owner: $owner, name: $repo) { - issues(filterBy: {since: $since}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { + issues(filterBy: {since: $since, states: OPEN}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { edges { node { ...issue From e8020077ae13abc062565f3e5ff845d346d9a47a Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 18 Nov 2019 10:34:25 -0800 Subject: [PATCH 21/28] Remove debut statement --- api/client.go | 1 - 1 file changed, 1 deletion(-) diff --git a/api/client.go b/api/client.go index 3eac38393..a3fa8e72a 100644 --- a/api/client.go +++ b/api/client.go @@ -38,7 +38,6 @@ func VerboseLog(out io.Writer) ClientOption { return func(tr http.RoundTripper) http.RoundTripper { return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { fmt.Fprintf(out, "> %s %s\n", req.Method, req.URL.RequestURI()) - fmt.Fprintf(out, "> BODY %s\n", req.Body) res, err := tr.RoundTrip(req) if err == nil { fmt.Fprintf(out, "< HTTP %s\n", res.Status) From 75a3496bf19c9144d1dc73327bf7c3dd4c795946 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 18 Nov 2019 11:05:43 -0800 Subject: [PATCH 22/28] Test flags --- command/issue_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/command/issue_test.go b/command/issue_test.go index be93304b6..09a55609f 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -68,6 +68,31 @@ func TestIssueList(t *testing.T) { } } +func TestIssueList_withFlags(t *testing.T) { + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(`{"data": {}}`)) // Since we are testing that the flags are passed, we don't care about the response + + _, err := test.RunCommand(RootCmd, "issue list -a probablyCher -l web,bug -s open") + if err != nil { + t.Errorf("error running command `issue list`: %v", err) + } + + bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) + reqBody := struct { + Variables struct { + Assignee string + Labels []string + States []string + } + }{} + json.Unmarshal(bodyBytes, &reqBody) + + eq(t, reqBody.Variables.Assignee, "probablyCher") + eq(t, reqBody.Variables.Labels, []string{"web", "bug"}) + eq(t, reqBody.Variables.States, []string{"OPEN"}) +} + func TestIssueView(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() From e5af5be94022c65266ad6b22473bd2eaaef77fbc Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 18 Nov 2019 11:09:00 -0800 Subject: [PATCH 23/28] Merge remote-tracking branch 'origin/master' into issue-update --- README.md | 17 ++ api/queries.go | 215 ++++++++++++++++++++- api/queries_issue.go | 6 +- command/issue.go | 2 +- command/pr.go | 144 +++++++++++++- command/pr_checkout_test.go | 364 ++++++++++++++++++++++++++++++++++++ command/pr_create.go | 223 ++++++++++++++++++++++ command/pr_create_test.go | 136 ++++++++++++++ command/pr_test.go | 2 +- command/root.go | 4 + command/testing.go | 14 ++ context/blank_context.go | 35 +++- context/remote.go | 10 + git/git.go | 58 +++--- git/git_test.go | 57 ++++++ go.mod | 5 +- go.sum | 22 +++ test/fixtures/createPr.json | 9 + test/fixtures/prList.json | 6 +- test/fixtures/repoId.json | 7 + test/helpers.go | 32 ++++ utils/color.go | 45 +++-- utils/prepare_cmd.go | 6 + 23 files changed, 1354 insertions(+), 65 deletions(-) create mode 100644 command/pr_checkout_test.go create mode 100644 command/pr_create.go create mode 100644 command/pr_create_test.go create mode 100644 git/git_test.go create mode 100644 test/fixtures/createPr.json create mode 100644 test/fixtures/repoId.json diff --git a/README.md b/README.md index ca23a060d..47f80bd8b 100644 --- a/README.md +++ b/README.md @@ -4,8 +4,25 @@ The #ce-cli team is working on a publicly available CLI tool to reduce the frict This tool is an endeavor separate from [github/hub](https://github.com/github/hub), which acts as a proxy to `git`, since our aim is to reimagine from scratch the kind of command line interface to GitHub that would serve our users' interests best. +# Installation + +_warning, gh is in a very alpha phase_ + +`brew install github/gh/gh` + +That's it. You are now ready to use `gh` on the command line. 🥳 + # Process - [Demo planning doc](https://docs.google.com/document/d/18ym-_xjFTSXe0-xzgaBn13Su7MEhWfLE5qSNPJV4M0A/edit) - [Weekly tracking issue](https://github.com/github/gh-cli/labels/tracking%20issue) - [Weekly sync notes](https://docs.google.com/document/d/1eUo9nIzXbC1DG26Y3dk9hOceLua2yFlwlvFPZ82MwHg/edit) + +# How to create a release + +This can all be done from your local terminal. + +1. `git tag 'vVERSION_NUMBER' # example git tag 'v0.0.1'` +2. `git push origin vVERSION_NUMBER` +3. Wait a few minutes for the build to run and CI to pass. Look at the [actions tab](https://github.com/github/gh-cli/actions) to check the progress. +4. Go to https://github.com/github/homebrew-gh/releases and look at the release diff --git a/api/queries.go b/api/queries.go index 68a9d14ba..4168113a8 100644 --- a/api/queries.go +++ b/api/queries.go @@ -16,6 +16,93 @@ type PullRequest struct { State string URL string HeadRefName string + + HeadRepositoryOwner struct { + Login string + } + HeadRepository struct { + Name string + DefaultBranchRef struct { + Name string + } + } + IsCrossRepository bool + MaintainerCanModify bool + + ReviewDecision string + + Commits struct { + Nodes []struct { + Commit struct { + StatusCheckRollup struct { + Contexts struct { + Nodes []struct { + State string + Conclusion string + } + } + } + } + } + } +} + +func (pr PullRequest) HeadLabel() string { + if pr.IsCrossRepository { + return fmt.Sprintf("%s:%s", pr.HeadRepositoryOwner.Login, pr.HeadRefName) + } + return pr.HeadRefName +} + +type PullRequestReviewStatus struct { + ChangesRequested bool + Approved bool + ReviewRequired bool +} + +func (pr *PullRequest) ReviewStatus() PullRequestReviewStatus { + status := PullRequestReviewStatus{} + switch pr.ReviewDecision { + case "CHANGES_REQUESTED": + status.ChangesRequested = true + case "APPROVED": + status.Approved = true + case "REVIEW_REQUIRED": + status.ReviewRequired = true + } + return status +} + +type PullRequestChecksStatus struct { + Pending int + Failing int + Passing int + Total int +} + +func (pr *PullRequest) ChecksStatus() (summary PullRequestChecksStatus) { + if len(pr.Commits.Nodes) == 0 { + return + } + commit := pr.Commits.Nodes[0].Commit + for _, c := range commit.StatusCheckRollup.Contexts.Nodes { + state := c.State + if state == "" { + state = c.Conclusion + } + switch state { + case "SUCCESS", "NEUTRAL", "SKIPPED": + summary.Passing++ + case "ERROR", "FAILURE", "CANCELLED", "TIMED_OUT", "ACTION_REQUIRED": + summary.Failing++ + case "EXPECTED", "QUEUED", "PENDING", "IN_PROGRESS": + summary.Pending++ + default: + panic(fmt.Errorf("unsupported status: %q", state)) + } + summary.Total++ + } + return } type Repo interface { @@ -43,19 +130,46 @@ func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername st } query := ` - fragment pr on PullRequest { - number - title - url - headRefName - } + fragment pr on PullRequest { + number + title + url + headRefName + headRefName + headRepositoryOwner { + login + } + isCrossRepository + commits(last: 1) { + nodes { + commit { + statusCheckRollup { + contexts(last: 100) { + nodes { + ...on StatusContext { + state + } + ...on CheckRun { + conclusion + } + } + } + } + } + } + } + } + fragment prWithReviews on PullRequest { + ...pr + reviewDecision + } query($owner: String!, $repo: String!, $headRefName: String!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) { repository(owner: $owner, name: $repo) { pullRequests(headRefName: $headRefName, states: OPEN, first: 1) { edges { node { - ...pr + ...prWithReviews } } } @@ -63,7 +177,7 @@ func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername st viewerCreated: search(query: $viewerQuery, type: ISSUE, first: $per_page) { edges { node { - ...pr + ...prWithReviews } } pageInfo { @@ -127,6 +241,48 @@ func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername st return &payload, nil } +func PullRequestByNumber(client *Client, ghRepo Repo, number int) (*PullRequest, error) { + type response struct { + Repository struct { + PullRequest PullRequest + } + } + + query := ` + query($owner: String!, $repo: String!, $pr_number: Int!) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $pr_number) { + headRefName + headRepositoryOwner { + login + } + headRepository { + name + defaultBranchRef { + name + } + } + isCrossRepository + maintainerCanModify + } + } + }` + + variables := map[string]interface{}{ + "owner": ghRepo.RepoOwner(), + "repo": ghRepo.RepoName(), + "pr_number": number, + } + + var resp response + err := client.GraphQL(query, variables, &resp) + if err != nil { + return nil, err + } + + return &resp.Repository.PullRequest, nil +} + func PullRequestsForBranch(client *Client, ghRepo Repo, branch string) ([]PullRequest, error) { type response struct { Repository struct { @@ -173,6 +329,45 @@ func PullRequestsForBranch(client *Client, ghRepo Repo, branch string) ([]PullRe return prs, nil } +func CreatePullRequest(client *Client, ghRepo Repo, params map[string]interface{}) (*PullRequest, error) { + repoID, err := GitHubRepoId(client, ghRepo) + if err != nil { + return nil, err + } + + query := ` + mutation CreatePullRequest($input: CreatePullRequestInput!) { + createPullRequest(input: $input) { + pullRequest { + url + } + } + }` + + inputParams := map[string]interface{}{ + "repositoryId": repoID, + } + for key, val := range params { + inputParams[key] = val + } + variables := map[string]interface{}{ + "input": inputParams, + } + + result := struct { + CreatePullRequest struct { + PullRequest PullRequest + } + }{} + + err = client.GraphQL(query, variables, &result) + if err != nil { + return nil, err + } + + return &result.CreatePullRequest.PullRequest, nil +} + func PullRequestList(client *Client, vars map[string]interface{}, limit int) ([]PullRequest, error) { type response struct { Repository struct { @@ -214,6 +409,10 @@ func PullRequestList(client *Client, vars map[string]interface{}, limit int) ([] state url headRefName + headRepositoryOwner { + login + } + isCrossRepository } } pageInfo { diff --git a/api/queries_issue.go b/api/queries_issue.go index bbf9b276c..5f3d243ec 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -92,7 +92,7 @@ func IssueStatus(client *Client, ghRepo Repo, currentUsername string) (*IssuesPa } query($owner: String!, $repo: String!, $since: DateTime!, $viewer: String!, $per_page: Int = 10) { assigned: repository(owner: $owner, name: $repo) { - issues(filterBy: {assignee: $viewer}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { + issues(filterBy: {assignee: $viewer, states: OPEN}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { edges { node { ...issue @@ -101,7 +101,7 @@ func IssueStatus(client *Client, ghRepo Repo, currentUsername string) (*IssuesPa } } mentioned: repository(owner: $owner, name: $repo) { - issues(filterBy: {mentioned: $viewer}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { + issues(filterBy: {mentioned: $viewer, states: OPEN}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { edges { node { ...issue @@ -110,7 +110,7 @@ func IssueStatus(client *Client, ghRepo Repo, currentUsername string) (*IssuesPa } } recent: repository(owner: $owner, name: $repo) { - issues(filterBy: {since: $since}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { + issues(filterBy: {since: $since, states: OPEN}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { edges { node { ...issue diff --git a/command/issue.go b/command/issue.go index 9dfcd502c..ace7ac2e3 100644 --- a/command/issue.go +++ b/command/issue.go @@ -24,7 +24,7 @@ func init() { &cobra.Command{ Use: "view ", Args: cobra.MinimumNArgs(1), - Short: "Open an issue in the browser", + Short: "View an issue in the browser", RunE: issueView, }, ) diff --git a/command/pr.go b/command/pr.go index efbf8d995..1ce061dc4 100644 --- a/command/pr.go +++ b/command/pr.go @@ -3,9 +3,11 @@ package command import ( "fmt" "os" + "os/exec" "strconv" "github.com/github/gh-cli/api" + "github.com/github/gh-cli/git" "github.com/github/gh-cli/utils" "github.com/spf13/cobra" "golang.org/x/crypto/ssh/terminal" @@ -13,6 +15,8 @@ import ( func init() { RootCmd.AddCommand(prCmd) + prCmd.AddCommand(prCheckoutCmd) + prCmd.AddCommand(prCreateCmd) prCmd.AddCommand(prListCmd) prCmd.AddCommand(prStatusCmd) prCmd.AddCommand(prViewCmd) @@ -28,6 +32,12 @@ var prCmd = &cobra.Command{ Short: "Work with pull requests", Long: `Helps you work with pull requests.`, } +var prCheckoutCmd = &cobra.Command{ + Use: "checkout ", + Short: "check out a pull request in git", + Args: cobra.MinimumNArgs(1), + RunE: prCheckout, +} var prListCmd = &cobra.Command{ Use: "list", Short: "List pull requests", @@ -40,7 +50,7 @@ var prStatusCmd = &cobra.Command{ } var prViewCmd = &cobra.Command{ Use: "view [pr-number]", - Short: "Open a pull request in the browser", + Short: "View a pull request in the browser", RunE: prView, } @@ -199,10 +209,10 @@ func prList(cmd *cobra.Command, args []string) error { case "MERGED": prNum = utils.Magenta(prNum) } - prBranch := utils.Cyan(truncate(branchWidth, pr.HeadRefName)) + prBranch := utils.Cyan(truncate(branchWidth, pr.HeadLabel())) fmt.Fprintf(out, "%s %-*s %s\n", prNum, titleWidth, truncate(titleWidth, pr.Title), prBranch) } else { - fmt.Fprintf(out, "%d\t%s\t%s\n", pr.Number, pr.Title, pr.HeadRefName) + fmt.Fprintf(out, "%d\t%s\t%s\n", pr.Number, pr.Title, pr.HeadLabel()) } } return nil @@ -246,9 +256,135 @@ func prView(cmd *cobra.Command, args []string) error { return utils.OpenInBrowser(openURL) } +func prCheckout(cmd *cobra.Command, args []string) error { + prNumber, err := strconv.Atoi(args[0]) + if err != nil { + return err + } + + 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 := api.PullRequestByNumber(apiClient, baseRemote, prNumber) + 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", prNumber) + 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(prs ...api.PullRequest) { for _, pr := range prs { - fmt.Printf(" #%d %s %s\n", pr.Number, truncate(50, pr.Title), utils.Cyan("["+pr.HeadRefName+"]")) + prNumber := fmt.Sprintf("#%d", pr.Number) + fmt.Printf(" %s %s %s", utils.Yellow(prNumber), truncate(50, pr.Title), utils.Cyan("["+pr.HeadLabel()+"]")) + + checks := pr.ChecksStatus() + reviews := pr.ReviewStatus() + if checks.Total > 0 || reviews.ChangesRequested || reviews.Approved { + fmt.Printf("\n ") + } + + if checks.Total > 0 { + var ratio string + if checks.Failing > 0 { + ratio = fmt.Sprintf("%d/%d", checks.Passing, checks.Total) + ratio = utils.Red(ratio) + } else if checks.Pending > 0 { + ratio = fmt.Sprintf("%d/%d", checks.Passing, checks.Total) + ratio = utils.Yellow(ratio) + } else if checks.Passing == checks.Total { + ratio = fmt.Sprintf("%d", checks.Total) + ratio = utils.Green(ratio) + } + fmt.Printf(" - checks: %s", ratio) + } + + if reviews.ChangesRequested { + fmt.Printf(" - %s", utils.Red("changes requested")) + } else if reviews.ReviewRequired { + fmt.Printf(" - %s", utils.Yellow("review required")) + } else if reviews.Approved { + fmt.Printf(" - %s", utils.Green("approved")) + } + + fmt.Printf("\n") } } diff --git a/command/pr_checkout_test.go b/command/pr_checkout_test.go new file mode 100644 index 000000000..930d2b4d8 --- /dev/null +++ b/command/pr_checkout_test.go @@ -0,0 +1,364 @@ +package command + +import ( + "bytes" + "os/exec" + "strings" + "testing" + + "github.com/github/gh-cli/context" + "github.com/github/gh-cli/utils" +) + +func TestPRCheckout_sameRepo(t *testing.T) { + 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": { "repository": { "pullRequest": { + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": false, + "maintainerCanModify": false + } } } } + `)) + + 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": + return &errorStub{"exit status: 1"} + default: + ranCommands = append(ranCommands, cmd.Args) + return &outputStub{} + } + }) + defer restoreCmd() + + RootCmd.SetArgs([]string{"pr", "checkout", "123"}) + _, err := prCheckoutCmd.ExecuteC() + eq(t, err, nil) + + 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") +} + +func TestPRCheckout_existingBranch(t *testing.T) { + 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": { "repository": { "pullRequest": { + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": false, + "maintainerCanModify": false + } } } } + `)) + + 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": + return &outputStub{} + default: + ranCommands = append(ranCommands, cmd.Args) + return &outputStub{} + } + }) + defer restoreCmd() + + RootCmd.SetArgs([]string{"pr", "checkout", "123"}) + _, err := prCheckoutCmd.ExecuteC() + eq(t, err, nil) + + 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[2], " "), "git merge --ff-only refs/remotes/origin/feature") +} + +func TestPRCheckout_differentRepo_remoteExists(t *testing.T) { + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + "robot-fork": "hubot/REPO", + }) + initContext = func() context.Context { + return ctx + } + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequest": { + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": true, + "maintainerCanModify": false + } } } } + `)) + + 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": + return &errorStub{"exit status: 1"} + default: + ranCommands = append(ranCommands, cmd.Args) + return &outputStub{} + } + }) + defer restoreCmd() + + RootCmd.SetArgs([]string{"pr", "checkout", "123"}) + _, err := prCheckoutCmd.ExecuteC() + eq(t, err, nil) + + 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") +} + +func TestPRCheckout_differentRepo(t *testing.T) { + 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": { "repository": { "pullRequest": { + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": true, + "maintainerCanModify": false + } } } } + `)) + + ranCommands := [][]string{} + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + switch strings.Join(cmd.Args, " ") { + case "git config branch.feature.merge": + return &errorStub{"exit status 1"} + default: + ranCommands = append(ranCommands, cmd.Args) + return &outputStub{} + } + }) + defer restoreCmd() + + RootCmd.SetArgs([]string{"pr", "checkout", "123"}) + _, err := prCheckoutCmd.ExecuteC() + eq(t, err, nil) + + 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") +} + +func TestPRCheckout_differentRepo_existingBranch(t *testing.T) { + 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": { "repository": { "pullRequest": { + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": true, + "maintainerCanModify": false + } } } } + `)) + + ranCommands := [][]string{} + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + switch strings.Join(cmd.Args, " ") { + case "git config branch.feature.merge": + return &outputStub{[]byte("refs/heads/feature\n")} + default: + ranCommands = append(ranCommands, cmd.Args) + return &outputStub{} + } + }) + defer restoreCmd() + + RootCmd.SetArgs([]string{"pr", "checkout", "123"}) + _, err := prCheckoutCmd.ExecuteC() + eq(t, err, nil) + + 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") +} + +func TestPRCheckout_differentRepo_currentBranch(t *testing.T) { + ctx := context.NewBlank() + ctx.SetBranch("feature") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequest": { + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": true, + "maintainerCanModify": false + } } } } + `)) + + ranCommands := [][]string{} + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + switch strings.Join(cmd.Args, " ") { + case "git config branch.feature.merge": + return &outputStub{[]byte("refs/heads/feature\n")} + default: + ranCommands = append(ranCommands, cmd.Args) + return &outputStub{} + } + }) + defer restoreCmd() + + RootCmd.SetArgs([]string{"pr", "checkout", "123"}) + _, err := prCheckoutCmd.ExecuteC() + eq(t, err, nil) + + eq(t, len(ranCommands), 2) + eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head") + eq(t, strings.Join(ranCommands[1], " "), "git merge --ff-only FETCH_HEAD") +} + +func TestPRCheckout_maintainerCanModify(t *testing.T) { + 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": { "repository": { "pullRequest": { + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": true, + "maintainerCanModify": true + } } } } + `)) + + ranCommands := [][]string{} + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + switch strings.Join(cmd.Args, " ") { + case "git config branch.feature.merge": + return &errorStub{"exit status 1"} + default: + ranCommands = append(ranCommands, cmd.Args) + return &outputStub{} + } + }) + defer restoreCmd() + + RootCmd.SetArgs([]string{"pr", "checkout", "123"}) + _, err := prCheckoutCmd.ExecuteC() + eq(t, err, nil) + + 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") +} diff --git a/command/pr_create.go b/command/pr_create.go new file mode 100644 index 000000000..c8d86f53a --- /dev/null +++ b/command/pr_create.go @@ -0,0 +1,223 @@ +package command + +import ( + "fmt" + "os" + "runtime" + + "github.com/AlecAivazis/survey/v2" + "github.com/github/gh-cli/api" + "github.com/github/gh-cli/context" + "github.com/github/gh-cli/git" + "github.com/pkg/errors" + "github.com/spf13/cobra" +) + +func prCreate(cmd *cobra.Command, _ []string) error { + ctx := contextForCommand(cmd) + + ucc, err := git.UncommittedChangeCount() + if err != nil { + return err + } + if ucc > 0 { + noun := "change" + if ucc > 1 { + // TODO: use pluralize helper + noun = noun + "s" + } + + cmd.Printf("Warning: %d uncommitted %s\n", ucc, noun) + } + + head, err := ctx.Branch() + if err != nil { + return errors.Wrap(err, "could not determine current branch") + } + + remote, err := guessRemote(ctx) + if err != nil { + return err + } + + if err = git.Push(remote, fmt.Sprintf("HEAD:%s", head)); err != nil { + return fmt.Errorf("was not able to push to remote '%s': %s", remote, err) + } + + title, err := cmd.Flags().GetString("title") + if err != nil { + return errors.Wrap(err, "could not parse title") + } + body, err := cmd.Flags().GetString("body") + if err != nil { + return errors.Wrap(err, "could not parse body") + } + + interactive := title == "" || body == "" + + inProgress := struct { + Body string + Title string + }{} + + if interactive { + confirmed := false + editor := determineEditor() + + for !confirmed { + titleQuestion := &survey.Question{ + Name: "title", + Prompt: &survey.Input{ + Message: "PR Title", + Default: inProgress.Title, + }, + } + bodyQuestion := &survey.Question{ + Name: "body", + Prompt: &survey.Editor{ + Message: fmt.Sprintf("PR Body (%s)", editor), + FileName: "*.md", + Default: inProgress.Body, + AppendDefault: true, + Editor: editor, + }, + } + + qs := []*survey.Question{} + if title == "" { + qs = append(qs, titleQuestion) + } + if body == "" { + qs = append(qs, bodyQuestion) + } + + err := survey.Ask(qs, &inProgress) + if err != nil { + return errors.Wrap(err, "could not prompt") + } + confirmAnswers := struct { + Confirmation string + }{} + confirmQs := []*survey.Question{ + { + Name: "confirmation", + Prompt: &survey.Select{ + Message: "Submit?", + Options: []string{ + "Yes", + "Edit", + "Cancel", + }, + }, + }, + } + + err = survey.Ask(confirmQs, &confirmAnswers) + if err != nil { + return errors.Wrap(err, "could not prompt") + } + + switch confirmAnswers.Confirmation { + case "Yes": + confirmed = true + case "Edit": + continue + case "Cancel": + cmd.Println("Discarding pull request") + return nil + } + } + } + + if title == "" { + title = inProgress.Title + } + if body == "" { + body = inProgress.Body + } + base, err := cmd.Flags().GetString("base") + if err != nil { + return errors.Wrap(err, "could not parse base") + } + if base == "" { + // TODO: use default branch for the repo + base = "master" + } + + client, err := apiClientForContext(ctx) + if err != nil { + return errors.Wrap(err, "could not initialize api client") + } + + repo, err := ctx.BaseRepo() + if err != nil { + return errors.Wrap(err, "could not determine GitHub repo") + } + + isDraft, err := cmd.Flags().GetBool("draft") + if err != nil { + return errors.Wrap(err, "could not parse draft") + } + + params := map[string]interface{}{ + "title": title, + "body": body, + "draft": isDraft, + "baseRefName": base, + "headRefName": head, + } + + pr, err := api.CreatePullRequest(client, repo, params) + if err != nil { + return errors.Wrap(err, "failed to create PR") + } + + fmt.Fprintln(cmd.OutOrStdout(), pr.URL) + return nil +} + +func guessRemote(ctx context.Context) (string, error) { + remotes, err := ctx.Remotes() + if err != nil { + return "", errors.Wrap(err, "could not read git remotes") + } + + // TODO: consolidate logic with fsContext.BaseRepo + // TODO: check if the GH repo that the remote points to is writeable + remote, err := remotes.FindByName("upstream", "github", "origin", "*") + if err != nil { + return "", errors.Wrap(err, "could not determine suitable remote") + } + + return remote.Name, nil +} + +func determineEditor() string { + if runtime.GOOS == "windows" { + return "notepad" + } + if v := os.Getenv("VISUAL"); v != "" { + return v + } + if e := os.Getenv("EDITOR"); e != "" { + return e + } + return "nano" +} + +var prCreateCmd = &cobra.Command{ + Use: "create", + Short: "Create a pull request", + RunE: prCreate, +} + +func init() { + prCreateCmd.Flags().BoolP("draft", "d", false, + "Mark PR as a draft") + prCreateCmd.Flags().StringP("title", "t", "", + "Supply a title. Will prompt for one otherwise.") + prCreateCmd.Flags().StringP("body", "b", "", + "Supply a body. Will prompt for one otherwise.") + prCreateCmd.Flags().StringP("base", "T", "", + "The branch into which you want your code merged") +} diff --git a/command/pr_create_test.go b/command/pr_create_test.go new file mode 100644 index 000000000..191043be3 --- /dev/null +++ b/command/pr_create_test.go @@ -0,0 +1,136 @@ +package command + +import ( + "bytes" + "encoding/json" + "fmt" + "io/ioutil" + "os" + "testing" + + "github.com/github/gh-cli/context" + "github.com/github/gh-cli/git" + "github.com/github/gh-cli/test" +) + +func TestPrCreateHelperProcess(*testing.T) { + if test.SkipTestHelperProcess() { + return + } + + args := test.GetTestHelperProcessArgs() + switch args[1] { + case "status": + switch args[0] { + case "clean": + case "dirty": + fmt.Println(" M git/git.go") + default: + fmt.Fprintf(os.Stderr, "unknown scenario: %q", args[0]) + os.Exit(1) + } + case "push": + default: + fmt.Fprintf(os.Stderr, "unknown command: %q", args[1]) + os.Exit(1) + } + os.Exit(0) +} + +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 + } + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "id": "REPOID" + } } } + `)) + 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 + }() + + out := bytes.Buffer{} + prCreateCmd.SetOut(&out) + + RootCmd.SetArgs([]string{"pr", "create", "-t", "mytitle", "-b", "mybody"}) + _, err := prCreateCmd.ExecuteC() + 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, "REPOID") + eq(t, reqBody.Variables.Input.Title, "mytitle") + eq(t, reqBody.Variables.Input.Body, "mybody") + eq(t, reqBody.Variables.Input.BaseRefName, "master") + eq(t, reqBody.Variables.Input.HeadRefName, "feature") + + eq(t, out.String(), "https://github.com/OWNER/REPO/pull/12\n") +} + +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 + } + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "id": "REPOID" + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "createPullRequest": { "pullRequest": { + "URL": "https://github.com/OWNER/REPO/pull/12" + } } } } + `)) + + origGitCommand := git.GitCommand + git.GitCommand = test.StubExecCommand("TestPrCreateHelperProcess", "dirty") + defer func() { + git.GitCommand = origGitCommand + }() + + out := bytes.Buffer{} + prCreateCmd.SetOut(&out) + + RootCmd.SetArgs([]string{"pr", "create", "-t", "mytitle", "-b", "mybody"}) + _, err := prCreateCmd.ExecuteC() + eq(t, err, nil) + + eq(t, out.String(), `Warning: 1 uncommitted change +https://github.com/OWNER/REPO/pull/12 +`) +} diff --git a/command/pr_test.go b/command/pr_test.go index 863632ee9..47851a5e8 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -66,7 +66,7 @@ func TestPRList(t *testing.T) { } eq(t, out.String(), `32 New feature feature -29 Fixed bad bug bug-fix +29 Fixed bad bug hubot:bug-fix 28 Improve documentation docs `) } diff --git a/command/root.go b/command/root.go index 1e9c449b6..4fe921391 100644 --- a/command/root.go +++ b/command/root.go @@ -72,6 +72,10 @@ var apiClientForContext = func(ctx context.Context) (*api.Client, error) { opts := []api.ClientOption{ api.AddHeader("Authorization", fmt.Sprintf("token %s", token)), api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version)), + // antiope-preview: Checks + // shadow-cat-preview: Draft pull requests + api.AddHeader("Accept", "application/vnd.github.antiope-preview+json, application/vnd.github.shadow-cat-preview"), + api.AddHeader("GraphQL-Features", "pe_mobile"), } if verbose := os.Getenv("DEBUG"); verbose != "" { opts = append(opts, api.VerboseLog(os.Stderr)) diff --git a/command/testing.go b/command/testing.go index 2e1cf9505..7758d4a8b 100644 --- a/command/testing.go +++ b/command/testing.go @@ -1,6 +1,8 @@ package command import ( + "errors" + "github.com/github/gh-cli/api" "github.com/github/gh-cli/context" ) @@ -34,3 +36,15 @@ func (s outputStub) Output() ([]byte, error) { func (s outputStub) Run() error { return nil } + +type errorStub struct { + message string +} + +func (s errorStub) Output() ([]byte, error) { + return nil, errors.New(s.message) +} + +func (s errorStub) Run() error { + return errors.New(s.message) +} diff --git a/context/blank_context.go b/context/blank_context.go index d5ad2cafe..733ef38a8 100644 --- a/context/blank_context.go +++ b/context/blank_context.go @@ -3,10 +3,12 @@ package context import ( "fmt" "strings" + + "github.com/github/gh-cli/git" ) // NewBlank initializes a blank Context suitable for testing -func NewBlank() Context { +func NewBlank() *blankContext { return &blankContext{} } @@ -16,6 +18,7 @@ type blankContext struct { authLogin string branch string baseRepo GitHubRepository + remotes Remotes } type ghRepo struct { @@ -54,14 +57,36 @@ func (c *blankContext) SetBranch(b string) { } func (c *blankContext) Remotes() (Remotes, error) { - return Remotes{}, nil + if c.remotes == nil { + return nil, fmt.Errorf("remotes were not initialized") + } + return c.remotes, nil +} + +func (c *blankContext) SetRemotes(stubs map[string]string) { + c.remotes = Remotes{} + for remoteName, repo := range stubs { + ownerWithName := strings.SplitN(repo, "/", 2) + c.remotes = append(c.remotes, &Remote{ + Remote: &git.Remote{Name: remoteName}, + Owner: ownerWithName[0], + Repo: ownerWithName[1], + }) + } } func (c *blankContext) BaseRepo() (GitHubRepository, error) { - if c.baseRepo == nil { - return nil, fmt.Errorf("base repo was not initialized") + if c.baseRepo != nil { + return c.baseRepo, nil } - return c.baseRepo, nil + remotes, err := c.Remotes() + if err != nil { + return nil, err + } + if len(remotes) < 1 { + return nil, fmt.Errorf("remotes are empty") + } + return remotes[0], nil } func (c *blankContext) SetBaseRepo(nwo string) { diff --git a/context/remote.go b/context/remote.go index 9f3b228dc..6c41eeaf1 100644 --- a/context/remote.go +++ b/context/remote.go @@ -25,6 +25,16 @@ func (r Remotes) FindByName(names ...string) (*Remote, error) { return nil, fmt.Errorf("no GitHub remotes found") } +// FindByRepo returns the first Remote that points to a specific GitHub repository +func (r Remotes) FindByRepo(owner, name string) (*Remote, error) { + for _, rem := range r { + if strings.EqualFold(rem.RepoOwner(), owner) && strings.EqualFold(rem.RepoName(), name) { + return rem, nil + } + } + return nil, fmt.Errorf("no matching remote found") +} + // Remote represents a git remote mapped to a GitHub repository type Remote struct { *git.Remote diff --git a/git/git.go b/git/git.go index a2a385f7f..8824872a4 100644 --- a/git/git.go +++ b/git/git.go @@ -33,7 +33,6 @@ func Dir() (string, error) { func WorkdirName() (string, error) { toplevelCmd := exec.Command("git", "rev-parse", "--show-toplevel") - toplevelCmd.Stderr = nil output, err := utils.PrepareCmd(toplevelCmd).Output() dir := firstLine(output) if dir == "" { @@ -42,31 +41,10 @@ func WorkdirName() (string, error) { return dir, err } -func HasFile(segments ...string) bool { - // The blessed way to resolve paths within git dir since Git 2.5.0 - pathCmd := exec.Command("git", "rev-parse", "-q", "--git-path", filepath.Join(segments...)) - if output, err := utils.PrepareCmd(pathCmd).Output(); err == nil { - if lines := outputLines(output); len(lines) == 1 { - if _, err := os.Stat(lines[0]); err == nil { - return true - } - } - } - - // Fallback for older git versions - dir, err := Dir() - if err != nil { - return false - } - - s := []string{dir} - s = append(s, segments...) - path := filepath.Join(s...) - if _, err := os.Stat(path); err == nil { - return true - } - - return false +func VerifyRef(ref string) bool { + showRef := exec.Command("git", "show-ref", "--verify", "--quiet", ref) + err := utils.PrepareCmd(showRef).Run() + return err == nil } func BranchAtRef(paths ...string) (name string, err error) { @@ -206,6 +184,34 @@ func LocalBranches() ([]string, error) { return branches, nil } +var GitCommand = func(args ...string) *exec.Cmd { + return exec.Command("git", args...) +} + +func UncommittedChangeCount() (int, error) { + statusCmd := GitCommand("status", "--porcelain") + output, err := utils.PrepareCmd(statusCmd).Output() + if err != nil { + return 0, err + } + lines := strings.Split(string(output), "\n") + + count := 0 + + for _, l := range lines { + if l != "" { + count++ + } + } + + return count, nil +} + +func Push(remote string, ref string) error { + cmd := GitCommand("push", "--set-upstream", remote, ref) + return cmd.Run() +} + func outputLines(output []byte) []string { lines := strings.TrimSuffix(string(output), "\n") return strings.Split(lines, "\n") diff --git a/git/git_test.go b/git/git_test.go new file mode 100644 index 000000000..dc1446c33 --- /dev/null +++ b/git/git_test.go @@ -0,0 +1,57 @@ +package git + +import ( + "fmt" + "os" + "strings" + "testing" + + "github.com/github/gh-cli/test" +) + +func TestGitStatusHelperProcess(*testing.T) { + if test.SkipTestHelperProcess() { + return + } + + args := test.GetTestHelperProcessArgs() + switch args[0] { + case "no changes": + case "one change": + fmt.Println(" M poem.txt") + case "untracked file": + fmt.Println(" M poem.txt") + fmt.Println("?? new.txt") + case "boom": + os.Exit(1) + } + os.Exit(0) +} + +func Test_UncommittedChangeCount(t *testing.T) { + origGitCommand := GitCommand + defer func() { + GitCommand = origGitCommand + }() + + cases := map[string]int{ + "no changes": 0, + "one change": 1, + "untracked file": 2, + } + + for k, v := range cases { + GitCommand = test.StubExecCommand("TestGitStatusHelperProcess", k) + ucc, _ := UncommittedChangeCount() + + if ucc != v { + t.Errorf("got unexpected ucc value: %d for case %s", ucc, k) + } + } + + GitCommand = test.StubExecCommand("TestGitStatusHelperProcess", "boom") + _, err := UncommittedChangeCount() + if !strings.HasSuffix(err.Error(), "git.test: exit status 1") { + t.Errorf("got unexpected error message: %s", err) + } +} diff --git a/go.mod b/go.mod index 9c89ff7b1..f4a0ea38b 100644 --- a/go.mod +++ b/go.mod @@ -3,12 +3,15 @@ module github.com/github/gh-cli go 1.13 require ( + github.com/AlecAivazis/survey/v2 v2.0.4 github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 github.com/mattn/go-colorable v0.1.2 github.com/mattn/go-isatty v0.0.9 github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b github.com/mitchellh/go-homedir v1.1.0 + github.com/pkg/errors v0.8.1 github.com/spf13/cobra v0.0.5 - golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9 + github.com/stretchr/testify v1.3.0 // indirect + golang.org/x/crypto v0.0.0-20190530122614-20be4c3c3ed5 gopkg.in/yaml.v3 v3.0.0-20191010095647-fc94e3f71652 ) diff --git a/go.sum b/go.sum index af7d46f56..b2f2e063e 100644 --- a/go.sum +++ b/go.sum @@ -1,18 +1,27 @@ +github.com/AlecAivazis/survey/v2 v2.0.4 h1:qzXnJSzXEvmUllWqMBWpZndvT2YfoAUzAMvZUax3L2M= +github.com/AlecAivazis/survey/v2 v2.0.4/go.mod h1:WYBhg6f0y/fNYUuesWQc0PKbJcEliGcYHB9sNT3Bg74= github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= +github.com/Netflix/go-expect v0.0.0-20180615182759-c93bf25de8e8 h1:xzYJEypr/85nBpB11F9br+3HUrpgb+fcm5iADzXXYEw= +github.com/Netflix/go-expect v0.0.0-20180615182759-c93bf25de8e8/go.mod h1:oX5x61PbNXchhh0oikYAH+4Pcfw5LKv21+Jnpr6r6Pc= github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8= github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= github.com/coreos/go-etcd v2.0.0+incompatible/go.mod h1:Jez6KQU2B/sWsbdaef3ED8NzMklzPG4d5KIOhIy30Tk= github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk= github.com/cpuguy83/go-md2man v1.0.10/go.mod h1:SmD6nW6nTyfqj6ABTjUi3V3JVMnlJmwcJI5acqYI6dE= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= +github.com/hinshun/vt10x v0.0.0-20180616224451-1954e6464174 h1:WlZsjVhE8Af9IcZDGgJGQpNflI3+MJSBhsgT5PCtzBQ= +github.com/hinshun/vt10x v0.0.0-20180616224451-1954e6464174/go.mod h1:DqJ97dSdRW1W22yXSB90986pcOyQ7r45iio1KN2ez1A= github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM= github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs= github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8= +github.com/kr/pty v1.1.4 h1:5Myjjh3JY/NaAi4IsUbHADytDyl1VE1Y9PXDlL+P/VQ= +github.com/kr/pty v1.1.4/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= github.com/mattn/go-colorable v0.1.2 h1:/bC9yWikZXAL9uJdulbSfyVNIR3n3trXl+v8+1sx8mU= github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= @@ -25,6 +34,8 @@ github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic= +github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= +github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g= @@ -36,13 +47,24 @@ github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb6 github.com/spf13/pflag v1.0.3 h1:zPAT6CGy6wXeQ7NtTnaTerfKOsV6V6F8agHXFiazDkg= github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DMA2s= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.2.1/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= +github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0= github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q= golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9 h1:mKdxBk7AujPs8kU4m80U72y/zjbZ3UcXC7dClwKbUI0= golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20190530122614-20be4c3c3ed5 h1:8dUaAV7K4uHsF56JQWkprecIQKdPHtR9jCHF5nB8uzc= +golang.org/x/crypto v0.0.0-20190530122614-20be4c3c3ed5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/sys v0.0.0-20181205085412-a5c9d58dba9a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20190530182044-ad28b68e88f1/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a h1:aYOabOQFp6Vj6W1F80affTUvO9UxmJRx8K0gsfABByQ= golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/test/fixtures/createPr.json b/test/fixtures/createPr.json new file mode 100644 index 000000000..47be76948 --- /dev/null +++ b/test/fixtures/createPr.json @@ -0,0 +1,9 @@ +{ + "data": { + "createPullRequest": { + "pullRequest": { + "url": "https://github.com/vilmibm/testing/pull/14" + } + } + } +} diff --git a/test/fixtures/prList.json b/test/fixtures/prList.json index 7ff9502a0..2808a5a8e 100644 --- a/test/fixtures/prList.json +++ b/test/fixtures/prList.json @@ -16,7 +16,11 @@ "number": 29, "title": "Fixed bad bug", "url": "https://github.com/monalisa/hello/pull/29", - "headRefName": "bug-fix" + "headRefName": "bug-fix", + "isCrossRepository": true, + "headRepositoryOwner": { + "login": "hubot" + } } }, { diff --git a/test/fixtures/repoId.json b/test/fixtures/repoId.json new file mode 100644 index 000000000..965b39f3d --- /dev/null +++ b/test/fixtures/repoId.json @@ -0,0 +1,7 @@ +{ + "data": { + "repository": { + "id": "a repo id" + } + } +} diff --git a/test/helpers.go b/test/helpers.go index 162befa56..dca930be5 100644 --- a/test/helpers.go +++ b/test/helpers.go @@ -11,6 +11,38 @@ import ( "github.com/spf13/cobra" ) +func GetTestHelperProcessArgs() []string { + args := os.Args + for len(args) > 0 { + if args[0] == "--" { + args = args[1:] + break + } + args = args[1:] + } + return args +} + +func SkipTestHelperProcess() bool { + return os.Getenv("GO_WANT_HELPER_PROCESS") != "1" +} + +func StubExecCommand(testHelper string, desiredOutput string) func(...string) *exec.Cmd { + return func(args ...string) *exec.Cmd { + cs := []string{ + fmt.Sprintf("-test.run=%s", testHelper), + "--", desiredOutput} + cs = append(cs, args...) + env := []string{ + "GO_WANT_HELPER_PROCESS=1", + } + + cmd := exec.Command(os.Args[0], cs...) + cmd.Env = append(env, os.Environ()...) + return cmd + } +} + type TempGitRepo struct { Remote string TearDown func() diff --git a/utils/color.go b/utils/color.go index 6fe6f2fc5..fb8479734 100644 --- a/utils/color.go +++ b/utils/color.go @@ -1,24 +1,39 @@ package utils -import "github.com/mgutz/ansi" +import ( + "github.com/mattn/go-isatty" + "github.com/mgutz/ansi" + "os" +) -var Black = ansi.ColorFunc("black") -var White = ansi.ColorFunc("white") +func makeColorFunc(color string) func(string) string { + return func(arg string) string { + output := arg + if isatty.IsTerminal(os.Stdout.Fd()) { + output = ansi.Color(color+arg+ansi.Reset, "") + } -func Gray(arg string) string { - return ansi.Color(ansi.LightBlack+arg, "") + return output + } } -var Red = ansi.ColorFunc("red") -var Green = ansi.ColorFunc("green") -var Yellow = ansi.ColorFunc("yellow") -var Blue = ansi.ColorFunc("blue") -var Magenta = ansi.ColorFunc("magenta") -var Cyan = ansi.ColorFunc("cyan") +var Black = makeColorFunc(ansi.Black) +var White = makeColorFunc(ansi.White) +var Magenta = makeColorFunc(ansi.Magenta) +var Cyan = makeColorFunc(ansi.Cyan) +var Red = makeColorFunc(ansi.Red) +var Yellow = makeColorFunc(ansi.Yellow) +var Blue = makeColorFunc(ansi.Blue) +var Green = makeColorFunc(ansi.Green) +var Gray = makeColorFunc(ansi.LightBlack) func Bold(arg string) string { - // This is really annoying. If you just define Bold as ColorFunc("+b") it will properly bold but - // will not use the default color, resulting in black and probably unreadable text. This forces - // the default color before bolding. - return ansi.Color(ansi.DefaultFG+arg, "+b") + output := arg + if isatty.IsTerminal(os.Stdout.Fd()) { + // This is really annoying. If you just define Bold as ColorFunc("+b") it will properly bold but + // will not use the default color, resulting in black and probably unreadable text. This forces + // the default color before bolding. + output = ansi.Color(ansi.DefaultFG+arg+ansi.Reset, "+b") + } + return output } diff --git a/utils/prepare_cmd.go b/utils/prepare_cmd.go index 0129d1a5b..6b354cb80 100644 --- a/utils/prepare_cmd.go +++ b/utils/prepare_cmd.go @@ -38,6 +38,9 @@ func (c cmdWithStderr) Output() ([]byte, error) { if os.Getenv("DEBUG") != "" { fmt.Fprintf(os.Stderr, "%v\n", c.Cmd.Args) } + if c.Cmd.Stderr != nil { + return c.Cmd.Output() + } errStream := &bytes.Buffer{} c.Cmd.Stderr = errStream out, err := c.Cmd.Output() @@ -51,6 +54,9 @@ func (c cmdWithStderr) Run() error { if os.Getenv("DEBUG") != "" { fmt.Fprintf(os.Stderr, "%v\n", c.Cmd.Args) } + if c.Cmd.Stderr != nil { + return c.Cmd.Run() + } errStream := &bytes.Buffer{} c.Cmd.Stderr = errStream err := c.Cmd.Run() From 20b47871a096643a37f62b87ba98f4e8f084903b Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 18 Nov 2019 14:56:15 -0800 Subject: [PATCH 24/28] Add labels to the issues status --- api/queries_issue.go | 108 ++++++++++++++++++++++--------------------- 1 file changed, 55 insertions(+), 53 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index 5f3d243ec..c990bda80 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -5,6 +5,59 @@ import ( "time" ) +type IssuesPayload struct { + Assigned []Issue + Mentioned []Issue + Recent []Issue +} + +type Issue struct { + Number int + Title string + URL string + Labels []string + TotalLabelCount int +} + +type apiIssues struct { + Issues struct { + Edges []struct { + Node struct { + Number int + Title string + URL string + Labels struct { + Edges []struct { + Node struct { + Name string + } + } + TotalCount int + } + } + } + } +} + +var fragments string + +func init() { + fragments = ` + fragment issue on Issue { + number + title + labels(first: 3) { + edges { + node { + name + } + } + totalCount + } + } + ` +} + func IssueCreate(client *Client, ghRepo Repo, params map[string]interface{}) (*Issue, error) { repoID, err := GitHubRepoId(client, ghRepo) if err != nil { @@ -44,40 +97,6 @@ func IssueCreate(client *Client, ghRepo Repo, params map[string]interface{}) (*I return &result.CreateIssue.Issue, nil } -type IssuesPayload struct { - Assigned []Issue - Mentioned []Issue - Recent []Issue -} - -type Issue struct { - Number int - Title string - URL string - Labels []string - TotalLabelCount int -} - -type apiIssues struct { - Issues struct { - Edges []struct { - Node struct { - Number int - Title string - URL string - Labels struct { - Edges []struct { - Node struct { - Name string - } - } - TotalCount int - } - } - } - } -} - func IssueStatus(client *Client, ghRepo Repo, currentUsername string) (*IssuesPayload, error) { type response struct { Assigned apiIssues @@ -85,11 +104,7 @@ func IssueStatus(client *Client, ghRepo Repo, currentUsername string) (*IssuesPa Recent apiIssues } - query := ` - fragment issue on Issue { - number - title - } + query := fragments + ` query($owner: String!, $repo: String!, $since: DateTime!, $viewer: String!, $per_page: Int = 10) { assigned: repository(owner: $owner, name: $repo) { issues(filterBy: {assignee: $viewer, states: OPEN}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { @@ -176,20 +191,7 @@ func IssueList(client *Client, ghRepo Repo, state string, labels []string, assig assignee = nil } - query := ` - fragment issue on Issue { - number - title - labels(first: 3) { - edges { - node { - name - } - } - totalCount - } - } - + query := fragments + ` query($owner: String!, $repo: String!, $limit: Int, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String) { repository(owner: $owner, name: $repo) { issues(first: $limit, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, labels: $labels, filterBy: {assignee: $assignee}) { From 5e7557e8fa37af26d3e3b97fd0db45e7c663227d Mon Sep 17 00:00:00 2001 From: Amanda Pinsker Date: Mon, 18 Nov 2019 23:26:18 -0800 Subject: [PATCH 25/28] Copy edits --- command/issue.go | 6 +++--- command/pr.go | 6 +++--- command/pr_create.go | 4 ++-- command/root.go | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/command/issue.go b/command/issue.go index ace7ac2e3..6c4b0597e 100644 --- a/command/issue.go +++ b/command/issue.go @@ -34,7 +34,7 @@ func init() { issueListCmd := &cobra.Command{ Use: "list", - Short: "List open issues", + Short: "List and filter issues in this repository", RunE: issueList, } issueListCmd.Flags().StringP("assignee", "a", "", "filter by assignee") @@ -46,8 +46,8 @@ func init() { var issueCmd = &cobra.Command{ Use: "issue", - Short: "Work with GitHub issues", - Long: `Helps you work with issues.`, + Short: "Work with issues", + Long: `Work with GitHub issues`, } var issueCreateCmd = &cobra.Command{ Use: "create", diff --git a/command/pr.go b/command/pr.go index 1ce061dc4..ab1ad4ad5 100644 --- a/command/pr.go +++ b/command/pr.go @@ -30,17 +30,17 @@ func init() { var prCmd = &cobra.Command{ Use: "pr", Short: "Work with pull requests", - Long: `Helps you work with pull requests.`, + Long: `Work with GitHub pull requests.`, } var prCheckoutCmd = &cobra.Command{ Use: "checkout ", - Short: "check out a pull request in git", + Short: "Check out a pull request in Git", Args: cobra.MinimumNArgs(1), RunE: prCheckout, } var prListCmd = &cobra.Command{ Use: "list", - Short: "List pull requests", + Short: "List and filter pull requests in this repository", RunE: prList, } var prStatusCmd = &cobra.Command{ diff --git a/command/pr_create.go b/command/pr_create.go index c8d86f53a..eebd7258d 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -68,14 +68,14 @@ func prCreate(cmd *cobra.Command, _ []string) error { titleQuestion := &survey.Question{ Name: "title", Prompt: &survey.Input{ - Message: "PR Title", + Message: "Pull request title", Default: inProgress.Title, }, } bodyQuestion := &survey.Question{ Name: "body", Prompt: &survey.Editor{ - Message: fmt.Sprintf("PR Body (%s)", editor), + Message: fmt.Sprintf("Pull request body (%s)", editor), FileName: "*.md", Default: inProgress.Body, AppendDefault: true, diff --git a/command/root.go b/command/root.go index 4fe921391..f2a73430d 100644 --- a/command/root.go +++ b/command/root.go @@ -37,7 +37,7 @@ type FlagError struct { var RootCmd = &cobra.Command{ Use: "gh", Short: "GitHub CLI", - Long: `Do things with GitHub from your terminal`, + Long: `Work with GitHub from your terminal`, SilenceErrors: true, SilenceUsage: true, From 7c731bc51289358c0cb14fb06087bcf2c5ec9d09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 19 Nov 2019 09:38:06 +0100 Subject: [PATCH 26/28] Avoid crash when parsing in-progress CheckRuns Fixes `panic: unsupported status: ""` This occurs when a CheckRun has status "IN_PROGRESS" (or any other than "COMPLETED") and when its `conclusion` would be null. I previously didn't account for this. This adds support for parsing state of an in-progress CheckRun. --- api/pull_request_test.go | 39 +++++++++++++++++++++++++++++++++++++++ api/queries.go | 13 ++++++++++--- 2 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 api/pull_request_test.go diff --git a/api/pull_request_test.go b/api/pull_request_test.go new file mode 100644 index 000000000..82386108d --- /dev/null +++ b/api/pull_request_test.go @@ -0,0 +1,39 @@ +package api + +import ( + "encoding/json" + "testing" +) + +func TestPullRequest_ChecksStatus(t *testing.T) { + pr := PullRequest{} + payload := ` + { "commits": { "nodes": [{ "commit": { + "statusCheckRollup": { + "contexts": { + "nodes": [ + { "state": "SUCCESS" }, + { "state": "PENDING" }, + { "state": "FAILURE" }, + { "status": "IN_PROGRESS", + "conclusion": null }, + { "status": "COMPLETED", + "conclusion": "SUCCESS" }, + { "status": "COMPLETED", + "conclusion": "FAILURE" }, + { "status": "COMPLETED", + "conclusion": "ACTION_REQUIRED" } + ] + } + } + } }] } } + ` + err := json.Unmarshal([]byte(payload), &pr) + eq(t, err, nil) + + checks := pr.ChecksStatus() + eq(t, checks.Total, 7) + eq(t, checks.Pending, 2) + eq(t, checks.Failing, 3) + eq(t, checks.Passing, 2) +} diff --git a/api/queries.go b/api/queries.go index 916e34153..2be64e46c 100644 --- a/api/queries.go +++ b/api/queries.go @@ -38,6 +38,7 @@ type PullRequest struct { Contexts struct { Nodes []struct { State string + Status string Conclusion string } } @@ -86,16 +87,21 @@ func (pr *PullRequest) ChecksStatus() (summary PullRequestChecksStatus) { } commit := pr.Commits.Nodes[0].Commit for _, c := range commit.StatusCheckRollup.Contexts.Nodes { - state := c.State + state := c.State // StatusContext if state == "" { - state = c.Conclusion + // CheckRun + if c.Status == "COMPLETED" { + state = c.Conclusion + } else { + state = c.Status + } } switch state { case "SUCCESS", "NEUTRAL", "SKIPPED": summary.Passing++ case "ERROR", "FAILURE", "CANCELLED", "TIMED_OUT", "ACTION_REQUIRED": summary.Failing++ - case "EXPECTED", "QUEUED", "PENDING", "IN_PROGRESS": + case "EXPECTED", "REQUESTED", "QUEUED", "PENDING", "IN_PROGRESS": summary.Pending++ default: panic(fmt.Errorf("unsupported status: %q", state)) @@ -150,6 +156,7 @@ func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername st state } ...on CheckRun { + status conclusion } } From c1ad55ba6425f07474602ac4b32cd7b8bc6b4757 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 19 Nov 2019 14:57:52 -0800 Subject: [PATCH 27/28] use const --- api/queries_issue.go | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index c990bda80..31793e608 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -39,24 +39,20 @@ type apiIssues struct { } } -var fragments string - -func init() { - fragments = ` - fragment issue on Issue { - number - title - labels(first: 3) { - edges { - node { - name - } +const fragments = ` + fragment issue on Issue { + number + title + labels(first: 3) { + edges { + node { + name } - totalCount } + totalCount } - ` -} + } +` func IssueCreate(client *Client, ghRepo Repo, params map[string]interface{}) (*Issue, error) { repoID, err := GitHubRepoId(client, ghRepo) From 89db5cbee62ee5185ad768192ac62800361d1679 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 19 Nov 2019 14:59:17 -0800 Subject: [PATCH 28/28] Move to queries_pr --- api/{queries.go => queries_pr.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename api/{queries.go => queries_pr.go} (100%) diff --git a/api/queries.go b/api/queries_pr.go similarity index 100% rename from api/queries.go rename to api/queries_pr.go