Skip to content

Commit a68cefa

Browse files
committed
Merge remote-tracking branch 'origin' into ghe-remotes
2 parents 446a411 + 6cc03d3 commit a68cefa

23 files changed

+839
-82
lines changed

Makefile

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,26 @@ ifdef SOURCE_DATE_EPOCH
88
else
99
BUILD_DATE ?= $(shell date "$(DATE_FMT)")
1010
endif
11-
LDFLAGS := -X github.com/cli/cli/command.Version=$(GH_VERSION) $(LDFLAGS)
12-
LDFLAGS := -X github.com/cli/cli/command.BuildDate=$(BUILD_DATE) $(LDFLAGS)
11+
12+
ifndef CGO_CPPFLAGS
13+
export CGO_CPPFLAGS := $(CPPFLAGS)
14+
endif
15+
ifndef CGO_CFLAGS
16+
export CGO_CFLAGS := $(CFLAGS)
17+
endif
18+
ifndef CGO_LDFLAGS
19+
export CGO_LDFLAGS := $(LDFLAGS)
20+
endif
21+
22+
GO_LDFLAGS := -X github.com/cli/cli/command.Version=$(GH_VERSION)
23+
GO_LDFLAGS := -X github.com/cli/cli/command.BuildDate=$(BUILD_DATE)
1324
ifdef GH_OAUTH_CLIENT_SECRET
14-
LDFLAGS := -X github.com/cli/cli/internal/config.oauthClientID=$(GH_OAUTH_CLIENT_ID) $(LDFLAGS)
15-
LDFLAGS := -X github.com/cli/cli/internal/config.oauthClientSecret=$(GH_OAUTH_CLIENT_SECRET) $(LDFLAGS)
25+
GO_LDFLAGS := -X github.com/cli/cli/internal/config.oauthClientID=$(GH_OAUTH_CLIENT_ID)
26+
GO_LDFLAGS := -X github.com/cli/cli/internal/config.oauthClientSecret=$(GH_OAUTH_CLIENT_SECRET)
1627
endif
1728

1829
bin/gh: $(BUILD_FILES)
19-
@go build -trimpath -ldflags "$(LDFLAGS)" -o "$@" ./cmd/gh
30+
@go build -trimpath -ldflags "$(GO_LDFLAGS)" -o "$@" ./cmd/gh
2031

2132
test:
2233
go test ./...

api/client.go

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"io"
88
"io/ioutil"
99
"net/http"
10+
"net/url"
1011
"regexp"
1112
"strings"
1213

@@ -154,7 +155,21 @@ func (gr GraphQLErrorResponse) Error() string {
154155
for _, e := range gr.Errors {
155156
errorMessages = append(errorMessages, e.Message)
156157
}
157-
return fmt.Sprintf("graphql error: '%s'", strings.Join(errorMessages, ", "))
158+
return fmt.Sprintf("GraphQL error: %s", strings.Join(errorMessages, "\n"))
159+
}
160+
161+
// HTTPError is an error returned by a failed API call
162+
type HTTPError struct {
163+
StatusCode int
164+
RequestURL *url.URL
165+
Message string
166+
}
167+
168+
func (err HTTPError) Error() string {
169+
if err.Message != "" {
170+
return fmt.Sprintf("HTTP %d: %s (%s)", err.StatusCode, err.Message, err.RequestURL)
171+
}
172+
return fmt.Sprintf("HTTP %d (%s)", err.StatusCode, err.RequestURL)
158173
}
159174

160175
// Returns whether or not scopes are present, appID, and error
@@ -283,22 +298,25 @@ func handleResponse(resp *http.Response, data interface{}) error {
283298
}
284299

285300
func handleHTTPError(resp *http.Response) error {
286-
var message string
287-
var parsedBody struct {
288-
Message string `json:"message"`
301+
httpError := HTTPError{
302+
StatusCode: resp.StatusCode,
303+
RequestURL: resp.Request.URL,
289304
}
305+
290306
body, err := ioutil.ReadAll(resp.Body)
291307
if err != nil {
292-
return err
308+
httpError.Message = err.Error()
309+
return httpError
293310
}
294-
err = json.Unmarshal(body, &parsedBody)
295-
if err != nil {
296-
message = string(body)
297-
} else {
298-
message = parsedBody.Message
311+
312+
var parsedBody struct {
313+
Message string `json:"message"`
314+
}
315+
if err := json.Unmarshal(body, &parsedBody); err == nil {
316+
httpError.Message = parsedBody.Message
299317
}
300318

301-
return fmt.Errorf("http error, '%s' failed (%d): '%s'", resp.Request.URL, resp.StatusCode, message)
319+
return httpError
302320
}
303321

304322
var jsonTypeRE = regexp.MustCompile(`[/+]json($|;)`)

api/client_test.go

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package api
22

33
import (
44
"bytes"
5+
"errors"
56
"io/ioutil"
67
"reflect"
78
"testing"
@@ -46,9 +47,15 @@ func TestGraphQLError(t *testing.T) {
4647
client := NewClient(ReplaceTripper(http))
4748

4849
response := struct{}{}
49-
http.StubResponse(200, bytes.NewBufferString(`{"errors":[{"message":"OH NO"}]}`))
50+
http.StubResponse(200, bytes.NewBufferString(`
51+
{ "errors": [
52+
{"message":"OH NO"},
53+
{"message":"this is fine"}
54+
]
55+
}`))
56+
5057
err := client.GraphQL("", nil, &response)
51-
if err == nil || err.Error() != "graphql error: 'OH NO'" {
58+
if err == nil || err.Error() != "GraphQL error: OH NO\nthis is fine" {
5259
t.Fatalf("got %q", err.Error())
5360
}
5461
}
@@ -66,3 +73,23 @@ func TestRESTGetDelete(t *testing.T) {
6673
err := client.REST("DELETE", "applications/CLIENTID/grant", r, nil)
6774
eq(t, err, nil)
6875
}
76+
77+
func TestRESTError(t *testing.T) {
78+
http := &httpmock.Registry{}
79+
client := NewClient(ReplaceTripper(http))
80+
81+
http.StubResponse(422, bytes.NewBufferString(`{"message": "OH NO"}`))
82+
83+
var httpErr HTTPError
84+
err := client.REST("DELETE", "repos/branch", nil, nil)
85+
if err == nil || !errors.As(err, &httpErr) {
86+
t.Fatalf("got %v", err)
87+
}
88+
89+
if httpErr.StatusCode != 422 {
90+
t.Errorf("expected status code 422, got %d", httpErr.StatusCode)
91+
}
92+
if httpErr.Error() != "HTTP 422: OH NO (https://api.github.com/repos/branch)" {
93+
t.Errorf("got %q", httpErr.Error())
94+
}
95+
}

api/queries_issue.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ func IssueStatus(client *Client, repo ghrepo.Interface, currentUsername string)
198198
return &payload, nil
199199
}
200200

201-
func IssueList(client *Client, repo ghrepo.Interface, state string, labels []string, assigneeString string, limit int, authorString string) (*IssuesAndTotalCount, error) {
201+
func IssueList(client *Client, repo ghrepo.Interface, state string, labels []string, assigneeString string, limit int, authorString string, mentionString string, milestoneString string) (*IssuesAndTotalCount, error) {
202202
var states []string
203203
switch state {
204204
case "open", "":
@@ -212,10 +212,10 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str
212212
}
213213

214214
query := fragments + `
215-
query($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String, $author: String) {
215+
query($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String, $author: String, $mention: String, $milestone: String) {
216216
repository(owner: $owner, name: $repo) {
217217
hasIssuesEnabled
218-
issues(first: $limit, after: $endCursor, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, labels: $labels, filterBy: {assignee: $assignee, createdBy: $author}) {
218+
issues(first: $limit, after: $endCursor, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, labels: $labels, filterBy: {assignee: $assignee, createdBy: $author, mentioned: $mention, milestone: $milestone}) {
219219
totalCount
220220
nodes {
221221
...issue
@@ -243,6 +243,12 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str
243243
if authorString != "" {
244244
variables["author"] = authorString
245245
}
246+
if mentionString != "" {
247+
variables["mention"] = mentionString
248+
}
249+
if milestoneString != "" {
250+
variables["milestone"] = milestoneString
251+
}
246252

247253
var response struct {
248254
Repository struct {

api/queries_issue_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func TestIssueList(t *testing.T) {
4040
`))
4141

4242
repo, _ := ghrepo.FromFullName("OWNER/REPO")
43-
_, err := IssueList(client, repo, "open", []string{}, "", 251, "")
43+
_, err := IssueList(client, repo, "open", []string{}, "", 251, "", "", "")
4444
if err != nil {
4545
t.Fatalf("unexpected error: %v", err)
4646
}

api/queries_pr.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -540,8 +540,12 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, hea
540540
headRepositoryOwner {
541541
login
542542
}
543+
headRepository {
544+
name
545+
}
543546
isCrossRepository
544547
isDraft
548+
maintainerCanModify
545549
reviewRequests(first: 100) {
546550
nodes {
547551
requestedReviewer {
@@ -1007,11 +1011,8 @@ func PullRequestReady(client *Client, repo ghrepo.Interface, pr *PullRequest) er
10071011
}
10081012

10091013
func BranchDeleteRemote(client *Client, repo ghrepo.Interface, branch string) error {
1010-
var response struct {
1011-
NodeID string `json:"node_id"`
1012-
}
10131014
path := fmt.Sprintf("repos/%s/%s/git/refs/heads/%s", repo.RepoOwner(), repo.RepoName(), branch)
1014-
return client.REST("DELETE", path, nil, &response)
1015+
return client.REST("DELETE", path, nil, nil)
10151016
}
10161017

10171018
func min(a, b int) int {

api/queries_pr_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
package api
2+
3+
import (
4+
"testing"
5+
6+
"github.com/cli/cli/internal/ghrepo"
7+
"github.com/cli/cli/pkg/httpmock"
8+
)
9+
10+
func TestBranchDeleteRemote(t *testing.T) {
11+
var tests = []struct {
12+
name string
13+
responseStatus int
14+
responseBody string
15+
expectError bool
16+
}{
17+
{
18+
name: "success",
19+
responseStatus: 204,
20+
responseBody: "",
21+
expectError: false,
22+
},
23+
{
24+
name: "error",
25+
responseStatus: 500,
26+
responseBody: `{"message": "oh no"}`,
27+
expectError: true,
28+
},
29+
}
30+
31+
for _, tt := range tests {
32+
t.Run(tt.name, func(t *testing.T) {
33+
http := &httpmock.Registry{}
34+
http.Register(httpmock.MatchAny, httpmock.StatusStringResponse(tt.responseStatus, tt.responseBody))
35+
36+
client := NewClient(ReplaceTripper(http))
37+
repo, _ := ghrepo.FromFullName("OWNER/REPO")
38+
39+
err := BranchDeleteRemote(client, repo, "branch")
40+
if (err != nil) != tt.expectError {
41+
t.Fatalf("unexpected result: %v", err)
42+
}
43+
})
44+
}
45+
}

command/config.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ var configSetCmd = &cobra.Command{
4848
Short: "Update configuration with a value for the given key",
4949
Example: heredoc.Doc(`
5050
$ gh config set editor vim
51+
$ gh config set editor "code --wait"
5152
`),
5253
Args: cobra.ExactArgs(2),
5354
RunE: configSet,

command/issue.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import (
2020
)
2121

2222
func init() {
23+
issueCmd.PersistentFlags().StringP("repo", "R", "", "Select another repository using the `OWNER/REPO` format")
24+
2325
RootCmd.AddCommand(issueCmd)
2426
issueCmd.AddCommand(issueStatusCmd)
2527

@@ -40,6 +42,8 @@ func init() {
4042
issueListCmd.Flags().StringP("state", "s", "open", "Filter by state: {open|closed|all}")
4143
issueListCmd.Flags().IntP("limit", "L", 30, "Maximum number of issues to fetch")
4244
issueListCmd.Flags().StringP("author", "A", "", "Filter by author")
45+
issueListCmd.Flags().String("mention", "", "Filter by mention")
46+
issueListCmd.Flags().StringP("milestone", "m", "", "Filter by milestone `name`")
4347

4448
issueCmd.AddCommand(issueViewCmd)
4549
issueViewCmd.Flags().BoolP("web", "w", false, "Open an issue in the browser")
@@ -154,15 +158,25 @@ func issueList(cmd *cobra.Command, args []string) error {
154158
return err
155159
}
156160

157-
listResult, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit, author)
161+
mention, err := cmd.Flags().GetString("mention")
162+
if err != nil {
163+
return err
164+
}
165+
166+
milestone, err := cmd.Flags().GetString("milestone")
167+
if err != nil {
168+
return err
169+
}
170+
171+
listResult, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit, author, mention, milestone)
158172
if err != nil {
159173
return err
160174
}
161175

162176
hasFilters := false
163177
cmd.Flags().Visit(func(f *pflag.Flag) {
164178
switch f.Name {
165-
case "state", "label", "assignee", "author":
179+
case "state", "label", "assignee", "author", "mention", "milestone":
166180
hasFilters = true
167181
}
168182
})

command/issue_test.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ func TestIssueList_withFlags(t *testing.T) {
153153
} } }
154154
`))
155155

156-
output, err := RunCommand("issue list -a probablyCher -l web,bug -s open -A foo")
156+
output, err := RunCommand("issue list -a probablyCher -l web,bug -s open -A foo --mention me --milestone 1.x")
157157
if err != nil {
158158
t.Errorf("error running command `issue list`: %v", err)
159159
}
@@ -167,10 +167,12 @@ No issues match your search in OWNER/REPO
167167
bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
168168
reqBody := struct {
169169
Variables struct {
170-
Assignee string
171-
Labels []string
172-
States []string
173-
Author string
170+
Assignee string
171+
Labels []string
172+
States []string
173+
Author string
174+
Mention string
175+
Milestone string
174176
}
175177
}{}
176178
_ = json.Unmarshal(bodyBytes, &reqBody)
@@ -179,6 +181,8 @@ No issues match your search in OWNER/REPO
179181
eq(t, reqBody.Variables.Labels, []string{"web", "bug"})
180182
eq(t, reqBody.Variables.States, []string{"OPEN"})
181183
eq(t, reqBody.Variables.Author, "foo")
184+
eq(t, reqBody.Variables.Mention, "me")
185+
eq(t, reqBody.Variables.Milestone, "1.x")
182186
}
183187

184188
func TestIssueList_withInvalidLimitFlag(t *testing.T) {
@@ -403,7 +407,7 @@ func TestIssueView_web_notFound(t *testing.T) {
403407
defer restoreCmd()
404408

405409
_, err := RunCommand("issue view -w 9999")
406-
if err == nil || err.Error() != "graphql error: 'Could not resolve to an Issue with the number of 9999.'" {
410+
if err == nil || err.Error() != "GraphQL error: Could not resolve to an Issue with the number of 9999." {
407411
t.Errorf("error running command `issue view`: %v", err)
408412
}
409413

0 commit comments

Comments
 (0)