search

rladies / meetupr

R interface to the meetup.com API
https://rladies.github.io/meetupr
MIT License
76 stars 25 forks source link

Fix #111 by using next_url only for pagination handling #113

Open GregSutcliffe opened 3 years ago

GregSutcliffe commented 3 years ago

As per discussion, I removed the initial call to meetup_call as it appeared to be duplicating results where offsetn == 2.

Since we now don't actually use i in the API calls (it's all handled by next_page) I've also taken the opportunity to make the code more readable: offfsetn is renamed to max_pages, and we start the loop from 2 so we're clear it's all the subsequent pages.

Also, I snuck in an extra commit for default values on organizer because I keep tripping over that - but I can break it out to another PR if you wish.

I'll take a look at the tests now, but I wanted to get this posted while it was fresh in my head.

GregSutcliffe commented 3 years ago

Tests added as per discussion in #112

GregSutcliffe commented 3 years ago

Ugh, tests have failed. They pass for me locally with testthat::test_file('tests/testthat/test-find_groups.R'), I will investigate...

GregSutcliffe commented 3 years ago

That's got it. Forgot part of a commit from my local branch :)

maelle commented 3 years ago

@ledell would you have time to have a look? :slightly_smiling_face: