Default per_page to 100. about pithub HOT 4 CLOSED

You're right, this is a better default. I'm gone for the weekend, but if you want to have a look at it in the meantime, I'm going to add you to plu/Pithub. So feel free to just push that change to master :)

from pithub.

Thanks. The change is trivial, but it perturbs the tests badly and will require test changes well outside its proportions. Mind you, this was just with setting per_page=30 to see what would happen without a behavioral change.

It potentially changes every URL (I think I can modify it so it only effects queries which use an iterator) which affects many tests. Checks for simple URL equality will have to be changed to take into account the per_page default as well as the unreliable ordering of the query keywords. I've written up url_is to handle this, and it might be worth adding to Test::URI, but I'm not sure where to put it. Pithub::Test would be the normal place, but that's already a testing subclass of Pithub.

The larger problem is with the response mocking. Adding a default query to every URL changes the response filenames which breaks a lot of tests. I could rename all the files to take into account the per_page=30 (and then change them all later 100). I could change Pithub::Test to strip out per_page if it's equal to the default (requiring Test knowledge of Pithub's defaults). Or something else?

Anyhow, that's a lot of test restructuring so I figure I'd get your input before launching off on it. I might not bother since I can just as easily set per_page in my application.

from pithub.

I've fixed one part of the problem (response stub filenames) and you can now use the Pithub::Test module for you intentions, if you want to.

I'm not sure where your code of url_is is living, I guess you have it somewhere? If you want to, you can fix that part, I'd be really happy about it. But I can also understand if you'd prefer that I clean up the mess I created myself :).

from pithub.

Done by #167

from pithub.