Skip to content

Commit 2ab27f5

Browse files
willnorriswillnorris
committed
use go-querystring to construct all request URLs
this is a breaking change for a handful of Option structs that were defining the Page option directly (they now embed ListOptions). finishes fixing #56
1 parent 7452bae commit 2ab27f5

16 files changed

+122
-234
lines changed

‎github/activity_events.go

Lines changed: 25 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ package github
88
import (
99
"encoding/json"
1010
"fmt"
11-
"net/url"
12-
"strconv"
1311
"time"
1412
)
1513

@@ -74,12 +72,9 @@ func (p PushEventCommit) String() string {
7472
//
7573
// GitHub API docs: http://developer.github.com/v3/activity/events/#list-public-events
7674
func (s *ActivityService) ListEvents(opt *ListOptions) ([]Event, *Response, error) {
77-
u := "events"
78-
if opt != nil {
79-
params := url.Values{
80-
"page": []string{strconv.Itoa(opt.Page)},
81-
}
82-
u += "?" + params.Encode()
75+
u, err := addOptions("events", opt)
76+
if err != nil {
77+
return nil, nil, err
8378
}
8479

8580
req, err := s.client.NewRequest("GET", u, nil)
@@ -101,11 +96,9 @@ func (s *ActivityService) ListEvents(opt *ListOptions) ([]Event, *Response, erro
10196
// GitHub API docs: http://developer.github.com/v3/activity/events/#list-repository-events
10297
func (s *ActivityService) ListRepositoryEvents(owner, repo string, opt *ListOptions) ([]Event, *Response, error) {
10398
u := fmt.Sprintf("repos/%v/%v/events", owner, repo)
104-
if opt != nil {
105-
params := url.Values{
106-
"page": []string{strconv.Itoa(opt.Page)},
107-
}
108-
u += "?" + params.Encode()
99+
u, err := addOptions(u, opt)
100+
if err != nil {
101+
return nil, nil, err
109102
}
110103

111104
req, err := s.client.NewRequest("GET", u, nil)
@@ -127,11 +120,9 @@ func (s *ActivityService) ListRepositoryEvents(owner, repo string, opt *ListOpti
127120
// GitHub API docs: http://developer.github.com/v3/activity/events/#list-issue-events-for-a-repository
128121
func (s *ActivityService) ListIssueEventsForRepository(owner, repo string, opt *ListOptions) ([]Event, *Response, error) {
129122
u := fmt.Sprintf("repos/%v/%v/issues/events", owner, repo)
130-
if opt != nil {
131-
params := url.Values{
132-
"page": []string{strconv.Itoa(opt.Page)},
133-
}
134-
u += "?" + params.Encode()
123+
u, err := addOptions(u, opt)
124+
if err != nil {
125+
return nil, nil, err
135126
}
136127

137128
req, err := s.client.NewRequest("GET", u, nil)
@@ -153,11 +144,9 @@ func (s *ActivityService) ListIssueEventsForRepository(owner, repo string, opt *
153144
// GitHub API docs: http://developer.github.com/v3/activity/events/#list-public-events-for-a-network-of-repositories
154145
func (s *ActivityService) ListEventsForRepoNetwork(owner, repo string, opt *ListOptions) ([]Event, *Response, error) {
155146
u := fmt.Sprintf("networks/%v/%v/events", owner, repo)
156-
if opt != nil {
157-
params := url.Values{
158-
"page": []string{strconv.Itoa(opt.Page)},
159-
}
160-
u += "?" + params.Encode()
147+
u, err := addOptions(u, opt)
148+
if err != nil {
149+
return nil, nil, err
161150
}
162151

163152
req, err := s.client.NewRequest("GET", u, nil)
@@ -179,11 +168,9 @@ func (s *ActivityService) ListEventsForRepoNetwork(owner, repo string, opt *List
179168
// GitHub API docs: http://developer.github.com/v3/activity/events/#list-public-events-for-an-organization
180169
func (s *ActivityService) ListEventsForOrganization(org string, opt *ListOptions) ([]Event, *Response, error) {
181170
u := fmt.Sprintf("orgs/%v/events", org)
182-
if opt != nil {
183-
params := url.Values{
184-
"page": []string{strconv.Itoa(opt.Page)},
185-
}
186-
u += "?" + params.Encode()
171+
u, err := addOptions(u, opt)
172+
if err != nil {
173+
return nil, nil, err
187174
}
188175

189176
req, err := s.client.NewRequest("GET", u, nil)
@@ -211,12 +198,9 @@ func (s *ActivityService) ListEventsPerformedByUser(user string, publicOnly bool
211198
} else {
212199
u = fmt.Sprintf("users/%v/events", user)
213200
}
214-
215-
if opt != nil {
216-
params := url.Values{
217-
"page": []string{strconv.Itoa(opt.Page)},
218-
}
219-
u += "?" + params.Encode()
201+
u, err := addOptions(u, opt)
202+
if err != nil {
203+
return nil, nil, err
220204
}
221205

222206
req, err := s.client.NewRequest("GET", u, nil)
@@ -244,12 +228,9 @@ func (s *ActivityService) ListEventsRecievedByUser(user string, publicOnly bool,
244228
} else {
245229
u = fmt.Sprintf("users/%v/received_events", user)
246230
}
247-
248-
if opt != nil {
249-
params := url.Values{
250-
"page": []string{strconv.Itoa(opt.Page)},
251-
}
252-
u += "?" + params.Encode()
231+
u, err := addOptions(u, opt)
232+
if err != nil {
233+
return nil, nil, err
253234
}
254235

255236
req, err := s.client.NewRequest("GET", u, nil)
@@ -272,12 +253,11 @@ func (s *ActivityService) ListEventsRecievedByUser(user string, publicOnly bool,
272253
// GitHub API docs: http://developer.github.com/v3/activity/events/#list-events-for-an-organization
273254
func (s *ActivityService) ListUserEventsForOrganization(org, user string, opt *ListOptions) ([]Event, *Response, error) {
274255
u := fmt.Sprintf("users/%v/events/orgs/%v", user, org)
275-
if opt != nil {
276-
params := url.Values{
277-
"page": []string{strconv.Itoa(opt.Page)},
278-
}
279-
u += "?" + params.Encode()
256+
u, err := addOptions(u, opt)
257+
if err != nil {
258+
return nil, nil, err
280259
}
260+
281261
req, err := s.client.NewRequest("GET", u, nil)
282262
if err != nil {
283263
return nil, nil, err

‎github/activity_star.go

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,25 +5,20 @@
55

66
package github
77

8-
import (
9-
"fmt"
10-
"net/url"
11-
"strconv"
12-
)
8+
import "fmt"
139

1410
// ActivityListStarredOptions specifies the optional parameters to the
1511
// ActivityService.ListStarred method.
1612
type ActivityListStarredOptions struct {
1713
// How to sort the repository list. Possible values are: created, updated,
1814
// pushed, full_name. Default is "full_name".
19-
Sort string
15+
Sort string `url:"sort,omitempty"`
2016

2117
// Direction in which to sort repositories. Possible values are: asc, desc.
2218
// Default is "asc" when sort is "full_name", otherwise default is "desc".
23-
Direction string
19+
Direction string `url:"direction,omitempty"`
2420

25-
// For paginated result sets, page of results to retrieve.
26-
Page int
21+
ListOptions
2722
}
2823

2924
// ListStarred lists all the repos starred by a user. Passing the empty string
@@ -37,14 +32,11 @@ func (s *ActivityService) ListStarred(user string, opt *ActivityListStarredOptio
3732
} else {
3833
u = "user/starred"
3934
}
40-
if opt != nil {
41-
params := url.Values{
42-
"sort": []string{opt.Sort},
43-
"direction": []string{opt.Direction},
44-
"page": []string{strconv.Itoa(opt.Page)},
45-
}
46-
u += "?" + params.Encode()
35+
u, err := addOptions(u, opt)
36+
if err != nil {
37+
return nil, nil, err
4738
}
39+
4840
req, err := s.client.NewRequest("GET", u, nil)
4941
if err != nil {
5042
return nil, nil, err

‎github/activity_star_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func TestActivityService_ListStarred_specifiedUser(t *testing.T) {
4646
fmt.Fprint(w, `[{"id":2}]`)
4747
})
4848

49-
opt := &ActivityListStarredOptions{"created", "asc", 2}
49+
opt := &ActivityListStarredOptions{"created", "asc", ListOptions{Page: 2}}
5050
repos, _, err := client.Activity.ListStarred("u", opt)
5151
if err != nil {
5252
t.Errorf("Activity.ListStarred returned error: %v", err)

‎github/gists.go

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package github
77

88
import (
99
"fmt"
10-
"net/url"
1110
"time"
1211
)
1312

@@ -56,7 +55,7 @@ func (g GistFile) String() string {
5655
// GistsService.List, GistsService.ListAll, and GistsService.ListStarred methods.
5756
type GistListOptions struct {
5857
// Since filters Gists by time.
59-
Since time.Time
58+
Since time.Time `url:"since,omitempty"`
6059
}
6160

6261
// List gists for a user. Passing the empty string will list
@@ -72,12 +71,9 @@ func (s *GistsService) List(user string, opt *GistListOptions) ([]Gist, *Respons
7271
} else {
7372
u = "gists"
7473
}
75-
if opt != nil {
76-
params := url.Values{}
77-
if !opt.Since.IsZero() {
78-
params.Add("since", opt.Since.Format(time.RFC3339))
79-
}
80-
u += "?" + params.Encode()
74+
u, err := addOptions(u, opt)
75+
if err != nil {
76+
return nil, nil, err
8177
}
8278

8379
req, err := s.client.NewRequest("GET", u, nil)
@@ -98,13 +94,9 @@ func (s *GistsService) List(user string, opt *GistListOptions) ([]Gist, *Respons
9894
//
9995
// GitHub API docs: http://developer.github.com/v3/gists/#list-gists
10096
func (s *GistsService) ListAll(opt *GistListOptions) ([]Gist, *Response, error) {
101-
u := "gists/public"
102-
if opt != nil {
103-
params := url.Values{}
104-
if !opt.Since.IsZero() {
105-
params.Add("since", opt.Since.Format(time.RFC3339))
106-
}
107-
u += "?" + params.Encode()
97+
u, err := addOptions("gists/public", opt)
98+
if err != nil {
99+
return nil, nil, err
108100
}
109101

110102
req, err := s.client.NewRequest("GET", u, nil)
@@ -125,13 +117,9 @@ func (s *GistsService) ListAll(opt *GistListOptions) ([]Gist, *Response, error)
125117
//
126118
// GitHub API docs: http://developer.github.com/v3/gists/#list-gists
127119
func (s *GistsService) ListStarred(opt *GistListOptions) ([]Gist, *Response, error) {
128-
u := "gists/starred"
129-
if opt != nil {
130-
params := url.Values{}
131-
if !opt.Since.IsZero() {
132-
params.Add("since", opt.Since.Format(time.RFC3339))
133-
}
134-
u += "?" + params.Encode()
120+
u, err := addOptions("gists/starred", opt)
121+
if err != nil {
122+
return nil, nil, err
135123
}
136124

137125
req, err := s.client.NewRequest("GET", u, nil)

‎github/issues.go

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,6 @@ package github
77

88
import (
99
"fmt"
10-
"net/url"
11-
"strconv"
12-
"strings"
1310
"time"
1411
)
1512

@@ -47,28 +44,27 @@ func (i Issue) String() string {
4744
type IssueListOptions struct {
4845
// Filter specifies which issues to list. Possible values are: assigned,
4946
// created, mentioned, subscribed, all. Default is "assigned".
50-
Filter string
47+
Filter string `url:"filter,omitempty"`
5148

5249
// State filters issues based on their state. Possible values are: open,
5350
// closed. Default is "open".
54-
State string
51+
State string `url:"state,omitempty"`
5552

5653
// Labels filters issues based on their label.
57-
Labels []string
54+
Labels []string `url:"labels,comma,omitempty"`
5855

5956
// Sort specifies how to sort issues. Possible values are: created, updated,
6057
// and comments. Default value is "assigned".
61-
Sort string
58+
Sort string `url:"sort,omitempty"`
6259

6360
// Direction in which to sort issues. Possible values are: asc, desc.
6461
// Default is "asc".
65-
Direction string
62+
Direction string `url:"direction,omitempty"`
6663

6764
// Since filters issues by time.
68-
Since time.Time
65+
Since time.Time `url:"since,omitempty"`
6966

70-
// For paginated result sets, page of results to retrieve.
71-
Page int
67+
ListOptions
7268
}
7369

7470
// List the issues for the authenticated user. If all is true, list issues
@@ -97,19 +93,9 @@ func (s *IssuesService) ListByOrg(org string, opt *IssueListOptions) ([]Issue, *
9793
}
9894

9995
func (s *IssuesService) listIssues(u string, opt *IssueListOptions) ([]Issue, *Response, error) {
100-
if opt != nil {
101-
params := url.Values{
102-
"filter": {opt.Filter},
103-
"state": {opt.State},
104-
"labels": {strings.Join(opt.Labels, ",")},
105-
"sort": {opt.Sort},
106-
"direction": {opt.Direction},
107-
"page": []string{strconv.Itoa(opt.Page)},
108-
}
109-
if !opt.Since.IsZero() {
110-
params.Add("since", opt.Since.Format(time.RFC3339))
111-
}
112-
u += "?" + params.Encode()
96+
u, err := addOptions(u, opt)
97+
if err != nil {
98+
return nil, nil, err
11399
}
114100

115101
req, err := s.client.NewRequest("GET", u, nil)
@@ -175,6 +161,7 @@ func (s *IssuesService) ListByRepo(owner string, repo string, opt *IssueListByRe
175161
if err != nil {
176162
return nil, nil, err
177163
}
164+
178165
req, err := s.client.NewRequest("GET", u, nil)
179166
if err != nil {
180167
return nil, nil, err
@@ -198,6 +185,7 @@ func (s *IssuesService) Get(owner string, repo string, number int) (*Issue, *Res
198185
if err != nil {
199186
return nil, nil, err
200187
}
188+
201189
issue := new(Issue)
202190
resp, err := s.client.Do(req, issue)
203191
if err != nil {
@@ -216,6 +204,7 @@ func (s *IssuesService) Create(owner string, repo string, issue *Issue) (*Issue,
216204
if err != nil {
217205
return nil, nil, err
218206
}
207+
219208
i := new(Issue)
220209
resp, err := s.client.Do(req, i)
221210
if err != nil {
@@ -234,6 +223,7 @@ func (s *IssuesService) Edit(owner string, repo string, number int, issue *Issue
234223
if err != nil {
235224
return nil, nil, err
236225
}
226+
237227
i := new(Issue)
238228
resp, err := s.client.Do(req, i)
239229
if err != nil {

0 commit comments

Comments
 (0)