search

bitfield / uptimerobot

Client library for UptimeRobot v2 API
MIT License
56 stars 11 forks source link

Add pagination to GetMonitors #14

Closed alandotcom closed 5 years ago

alandotcom commented 5 years ago

Fixes #1.

alandotcom commented 5 years ago

Tests added

bitfield commented 5 years ago

PS Don't forget to add 'Fixes #1', or something similar to the PR description. Then GitHub will close the issue automatically once it's merged.

alandotcom commented 5 years ago

@bitfield this doesn't fix #1, as it doesn't implement pagination for getAlertContacts

bitfield commented 5 years ago

Good point. OK, let’s make it 'Partly addressing #1", or similar.

bitfield commented 5 years ago

I've just merged the refactoring branch, which moves a lot of things around, so would you mind if I take this branch and rebase it on the new master? It's probably going to be a bit fiddly, so it'll be easier if I do it, since I know what I changed.

alandotcom commented 5 years ago

Yes please go ahead. I’m traveling this week and won’t be able to take a look until late next week. Thank you

On Thu, Aug 15, 2019 at 3:22 PM John Arundel notifications@github.com wrote:

I've just merged the refactoring branch, which moves a lot of things around, so would you mind if I take this branch and rebase it on the new master? It's probably going to be a bit fiddly, so it'll be easier if I do it, since I know what I changed.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bitfield/uptimerobot/pull/14?email_source=notifications&email_token=AAN2NZAF2O37DE2GL7FGPVTQEVKAJA5CNFSM4IGWCJKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4LZE3Y#issuecomment-521638511, or mute the thread https://github.com/notifications/unsubscribe-auth/AAN2NZAV3HTCSJ2JJXNHUFLQEVKAJANCNFSM4IGWCJKA .

bitfield commented 5 years ago

All sorted! Do you mind if I go ahead and make the suggested tweaks as well? If you seriously disagree with any of them, feel free to revert my commits and we'll talk it over.

bitfield commented 5 years ago

This looks really great! I tried it on an account with 300+ monitors, and it worked perfectly, returning all the monitors. Nice job. If you're happy with the branch, I'll merge it as soon as you give the go-ahead.

alandotcom commented 5 years ago

Go for it!

bitfield commented 5 years ago

Thanks again for this work, it's terrific! Much appreciated.