[jeanettehead]

See the discussion on: #406

Previous to this, the behavior is that specifying a page in options causes an infinite loop of requests and there is no way for calls that are delegated to requestAllPages to be paginated by the api user.

[clayreimann]

👍 I guess we missed this in our rewrite. Thanks for catching the bug!

[mtscout6]

@jeanettehead Can you add a test for this so future contributors don't remove this functionality out from under you by accident?

@clayreimann The CI is failing on master again for a change unrelated to this one.

[jeanettehead]

@mtscout6 I looked at testing it, but I didn't see any existing tests for the existing pagination behavior, or fixtures with paginated results. Is there something in there that I missed?

[mtscout6]

I don't think there is existing support for testing the explicit pagination that you are introducing. The other testing there is testing that all data is fetched, regardless of pagination.

[clayreimann]

@mtscout6 That builds are still failing is related to the flakeyness of github's APIs. To fully solve that problem we need to make sure that all of the tests are fully isolated and then configure the tests to retry a couple times when they fail.

[jeanettehead]

@mtscout6 any update on this?

I looked into testing it again, and I'm not really interested in implementing the setup required to test this one line change. The existing tests in user -> list repos are just testing that an array is returned - not that any particular data length comes back, and there no tests at all for the requestable object that was modified. Setting up a new test and fixtures for this seems like a lot relative to the change footprint.

[mtscout6]

Without tests there will be no guarantee that we don't accidentally remove this in the future. If your ok with that then ok.

[jeanettehead]

I'm OK with that.

[mtscout6]

@clayreimann I don't have permission to push this to npm. Can you do that as a minor version bump?

[clayreimann]

@mtscout6 I pushed a new minor version. The release script doesn't push to npm directly, we do that from our travis build so you should be able to release versions whenever you like.

[clayreimann]

e.g. npm run release -- minor

[clayreimann]

Looks like this broke the build 😢

[clayreimann]

@jeanettehead

[mtscout6]

@clayreimann Note that I mentioned earlier that the CI was already broken on master before she submitted this PR: #409 (comment)

[clayreimann]

Ahh, I guess I misunderstood.