Default :per_page parameter was being ignored

[gentilfp]

As mentioned here #137, parameter :per_page was being ignored if not passed directly in list method.

token = "9ac826391b6b8f4a680cef1ed2dfee1cc3192758"
github = Github.new(oauth_token: token, per_page: 2)
response = github.repos.list
response.count
=> 26

repos = Github::Client::Repos.new(per_page: 10)
response = repos.list(user: 'fpgentil', per_page: 2)
response.count
=> 2

repos = Github::Client::Repos.new(per_page: 2)
response = repos.list(user: 'fpgentil')
response.count
=> 25

repos = Github::Client::Repos.new
response = repos.list(user: 'fpgentil')
response.count
=> 25

This was my approach, feedbacks are always welcome :-)

[coveralls]

Coverage increased (+0.01%) to 97.53% when pulling eb7338a on fpgentil:fix-per-page into 031dff7 on peter-murach:master.

[coveralls]

Coverage decreased (-0.01%) to 97.51% when pulling 7354f5c on fpgentil:fix-per-page into 031dff7 on peter-murach:master.

[piotrmurach]

Sorry I don't have enough time to review properly now. Awesome that you have fixed the bug! My only comment would be about where the fix is actually applied. My intention is for the Arguments to be a simple parser that sifts request params but does nothing more. I cannot recall why autopagination ended up in this file. I would be inclined to keep anything api specific such as Configuration out of Arguments and push it down close to where the configuration options are processed etc... I will definitely work with you on that!

[gentilfp]

Thanks for the feedback, @peter-murach.
I spent some time figuring out where I should put this fix, the only reason was, like you mentioned, the autopagination. I totally agree these both configs should be somewhere else. I'll take some more time and see if I can find a solution to this problem and come back here with an update. If you have something to suggest, please let me know.

[gentilfp]

Hello @peter-murach, sorry it took so long for me to update this PR 🙈

Anyway, I have moved the logic into Pagination, it looks better to me, perhaps it might not be the best place, but I ran out of ideas 😆 Could you please let me know what's your opinion about it?

PS. I haven't done anything related to autopagination that shouldn't be either inside Arguments. Since it woks so far, I'll think about it later.

Cheers,

[coveralls]

Coverage increased (+0.09%) to 97.61% when pulling e7bce9f on fpgentil:fix-per-page into 031dff7 on peter-murach:master.

[coveralls]

Coverage increased (+0.09%) to 97.61% when pulling e7bce9f on fpgentil:fix-per-page into 031dff7 on peter-murach:master.

[coveralls]

Coverage increased (+0.09%) to 97.61% when pulling e7bce9f on fpgentil:fix-per-page into 031dff7 on peter-murach:master.

[coveralls]

Coverage increased (+0.09%) to 97.61% when pulling e7bce9f on fpgentil:fix-per-page into 031dff7 on peter-murach:master.

[coveralls]

Coverage decreased (-0.06%) to 97.46% when pulling e7bce9f on fpgentil:fix-per-page into 031dff7 on peter-murach:master.

[coveralls]

Coverage increased (+0.1%) to 97.62% when pulling e7bce9f on fpgentil:fix-per-page into 031dff7 on peter-murach:master.

[coveralls]

Coverage decreased (-0.06%) to 97.46% when pulling e7bce9f on fpgentil:fix-per-page into 031dff7 on peter-murach:master.

[andrewdotn]

I had a hard time tracking down a bug that was caused by this issue.

require 'github_api'

github = Github::Client.new do |config|
  config.auto_pagination = true
end

repos = github.repos.list(org: 'chef')
puts "Default per_page: #{repos.count} total repos"

github = Github::Client.new do |config|
  config.per_page = 100
  config.auto_pagination = true
end

repos = github.repos.list(org: 'chef')
puts "100 per_page: #{repos.count} total repos"

prints

Default per_page: 284 total repos
100 per_page: 214 total repos

@peter-murach @fpgentil Is there something I can do to help get this fix merged?