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

Move RateLimits method to a service #2968

Closed
WillAbides opened this issue Oct 19, 2023 · 3 comments · Fixed by #2969
Closed

Move RateLimits method to a service #2968

WillAbides opened this issue Oct 19, 2023 · 3 comments · Fixed by #2969
Assignees

Comments

@WillAbides
Copy link
Contributor

I missed the RateLimits method in #2937. It should be moved to a service struct.

Docs are at rate-limit/rate-limit#get-rate-limit-status-for-the-authenticated-user, so according to our existing pattern, we should move it to a new service called RateLimitService and name it Get

Client.RateLimits should be marked as deprecated and its body should be updated to return c.RateLimit.Get(ctx)

This may be a candidate for "good first issue" with the slight complication of creating a new service.

/cc #2936

@WillAbides
Copy link
Contributor Author

For whoever picks this up, remember to add the new service here and to move the RateLimits tests from github_test.go to ratelimit_test.go.

There will be a small decline in test coverage because the newly deprecated Client method is no longer tested. The decreased test coverage will cause a failure status on your PR, but that can safely be ignored as long as the decreased coverage is only the Client method.

@nhAnik
Copy link
Contributor

nhAnik commented Oct 19, 2023

Hi @WillAbides, can I work on this?

@gmlewis
Copy link
Collaborator

gmlewis commented Oct 19, 2023

Hi @WillAbides, can I work on this?

Thank you, @nhAnik ! It is yours.

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

Successfully merging a pull request may close this issue.

3 participants