Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ListContributorsStats and ListCommitActivity for RepositoriesService #94

Closed
wants to merge 1 commit into from

Conversation

ttacon
Copy link
Contributor

@ttacon ttacon commented Apr 8, 2014

When you make a request for stats, you often get a 202 back at first (meaning the stats haven't been computed but now will be computed). I didn't do any extra work because I figured the user would rather be the one handling this (once you receive a 202, waiting ~1 second and then making the request again results in a 200).

@@ -0,0 +1,72 @@
// Copyright 2013 The go-github AUTHORS. All rights reserved.
Copy link
Collaborator

Choose a reason for hiding this comment

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

go ahead and use the current year in newly created files

@willnorris
Copy link
Collaborator

I think all my comments are exclusively related to Go style or documentation. Otherwise, this looks great.

@willnorris
Copy link
Collaborator

Regarding the 202 vs 200 response, I think you did the right thing in the code here, namely leaving it to the caller to handle. Maybe we should add some documentation that explains that though?

@ttacon
Copy link
Contributor Author

ttacon commented Apr 8, 2014

Updated the comments and added comments to the functions about the 202 vs 200 response. I couldn't find anywhere else in the API (it was a quick skim) that used a unix timestamp so I left that alone.

@ttacon
Copy link
Contributor Author

ttacon commented Apr 8, 2014

Whoops! Forgot to update test (just did)!

@willnorris
Copy link
Collaborator

rebased onto master and merged as f9fd615. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants