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

Add helpers to access the scope and rate limit headers. #103

Merged
merged 3 commits into from
Apr 13, 2015
Merged

Add helpers to access the scope and rate limit headers. #103

merged 3 commits into from
Apr 13, 2015

Conversation

calavera
Copy link
Contributor

@calavera calavera commented Apr 7, 2015

No description provided.

return strings.Split(r.RawAcceptedScopes(), ",")
}

func (r *Result) ValidScope(scope string) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this logic be built into the application instead of octokit?

@owenthereal
Copy link
Contributor

In general, LGTM. Could add some tests though. @pengwynn What do you think?

func (r *Result) RateLimitRemaining() int {
rate, err := strconv.Atoi(r.Response.Header.Get(rateLimitRemaining))
if err != nil {
rate = 60
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is tricky, because Enterprise, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't remember anymore how that thing works 😆 I guess I can check the response host and set the default to 60 in the case of api.github.com

@pengwynn
Copy link
Collaborator

Thanks for the patch @calavera. 💖 I left a couple of notes.

@calavera
Copy link
Contributor Author

Thank you guys, I'll add some tests today.

…valid.

Add test for result helpers.

Signed-off-by: David Calavera <david.calavera@gmail.com>
@calavera
Copy link
Contributor Author

@pengwynn, @jingweno I've added some tests for these helpers. I don't remember what GitHub Enterprise returns as rate limit, but I've used -1 to indicate no limit. I take suggestions for a better default in that case.

@pengwynn
Copy link
Collaborator

I don't remember what GitHub Enterprise returns as rate limit,

It's currently not rate limited. I think -1 is a sane default in that case. Thanks for the patch, @calavera.

pengwynn added a commit that referenced this pull request Apr 13, 2015
Add helpers to access the scope and rate limit headers.
@pengwynn pengwynn merged commit 9046e96 into octokit:master Apr 13, 2015
@calavera calavera deleted the patch-1 branch April 13, 2015 22:13
@owenthereal
Copy link
Contributor

👍

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

Successfully merging this pull request may close these issues.

None yet

3 participants