Skip to content
This repository has been archived by the owner on Jun 20, 2022. It is now read-only.

Implemented Collaborators API using the new pattern #99

Merged
merged 10 commits into from
May 3, 2015

Conversation

dhruvsinghal
Copy link

@pengwynn please take a look

@dhruvsinghal dhruvsinghal self-assigned this Mar 23, 2015
@@ -0,0 +1,23 @@
package octokit
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned in Slack, I'd probably name this file repo_collaborators_test.go.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@pengwynn
Copy link
Collaborator

Looking good @dhruvsinghal. Gonna tackle the other GET and the mutating methods while you're in here?

@dhruvsinghal
Copy link
Author

Implemented the remaining GET call. @pengwynn Please take a look.

@@ -35,6 +35,12 @@ func (r *Request) Get(output interface{}) (*Response, error) {
return r.createResponse(r.Request.Get(), output)
}

// Get sends a GET request through the given client and returns the response
// and any associated errors. It does not fill in a response body.
func (r *Request) GetBodyless() (*Response, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not seeing why we need a new method here. Can we not make Get handle those types of responses?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@pengwynn
Copy link
Collaborator

Had one question on this one @dhruvsinghal. Ping me and add the needs-review label if you need me to take another look.

@dhruvsinghal
Copy link
Author

@pengwynn Please take a look.

@pengwynn pengwynn merged commit c802f41 into master May 3, 2015
@dhruvsinghal dhruvsinghal deleted the collaborators-api branch May 3, 2015 05:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants