Skip to content

Move RateLimits method to a service - #2969

Merged
gmlewis merged 2 commits into
google:masterfrom
nhAnik:move-rate-limit
Oct 20, 2023
Merged

Move RateLimits method to a service#2969
gmlewis merged 2 commits into
google:masterfrom
nhAnik:move-rate-limit

Conversation

@nhAnik

Copy link
Copy Markdown
Contributor

Closes#2968

@codecov

codecovBot commented Oct 20, 2023

Copy link
Copy Markdown

Codecov Report

Merging #2969 (64bfb14) into master (56ca0e5) will decrease coverage by 0.02%.
The diff coverage is 97.95%.

@@ Coverage Diff @@## master #2969 +/- ##
==========================================
- Coverage 98.13% 98.11% -0.02% 
==========================================
Files 148 149 +1 Lines 12787 12790 +3 ==========================================
+ Hits 12548 12549 +1 - Misses 164 166 +2 
Partials 75 75 
FilesCoverage Δ
github/rate_limit.go100.00% <100.00%> (ø)
github/github.go97.71% <50.00%> (-0.41%)⬇️

@gmlewisgmlewis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you, @nhAnik !
Anywhere you find client.RateLimits (such as test/integration/misc_test.go), it needs to be updated to call the new service instead.

@WillAbidesWillAbides left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This LGTM except for the lint failure. You can fix that by calling your new method instead of client.RateLimits

edit: looks like @gmlewis beat me to it :)

@WillAbidesWillAbides left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@gmlewisgmlewis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you, @nhAnik and @WillAbides !
LGTM.
Merging.

@gmlewis
gmlewis merged commit eea6e0a into google:masterOct 20, 2023
jlaportebot added a commit to jlaportebot/go-github that referenced this pull request Jun 28, 2026
Sign up for freeto 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.

Move RateLimits method to a service

3 participants

@nhAnik@WillAbides@gmlewis