Also concat files in paginated compare response

[tubaxenor]

Currently when paginating through compare results, we only concat commits, but files were left over and only contains results from page 1. This PR concats files to make sure result from later pages are also returned in final result set.

[timrogers]

@tubaxenor Thanks for this PR, and sorry that it has been very slow to get a review.

Have you tried this code yourself? I'm surprised that you need to take the files from every page, as the docs for the "Compare two commits" API say that "[w]hen using paging, the list of changed files is only returned with page 1, but includes all changed files for the entire comparison" (source).

[nickfloyd]

Hey @tubaxenor let me know if you're still interested in carrying this forward and/or your thoughts on @timrogers' comments. Thanks for being part of the community! ❤️

[tubaxenor]

Hi, I think this is still an issue and fairly easy to reproduce by the following code:

require 'octokit'

client = Octokit::Client.new(access_token: 'YOUR_TOKEN', auto_paginate: true, api_endpoint: 'https://api.github.com/')
result = client.compare('octokit/octokit.rb', '247d87aabc432cd2f386da4a72644f035cd335e8', 'main')

result.files.count
# => 300

result.total_commits
# => 1450

result.commits.count
# => 1450

From web UI: https://github.com/octokit/octokit.rb/compare/247d87aabc432cd2f386da4a72644f035cd335e8...main

So I don't think the API doc's statement is correct or at least it's not reflecting on octokit's implementation.

Even though I've already switched the files fetching differently, I do want to move forward and fix this issue, please let me know what's the process or step to follow and getting this PR ready to merge, thanks 🙇

[kfcampbell]

Hi @tubaxenor.

I'm able to run ./script/console on the main branch and reproduce the results you've given above (with changes for the updates to main). However, pulling down this branch and attempting the same command gives me the following error:

irb(main):008:0> result = client.compare('octokit/octokit.rb', '247d87aabc432cd2f386da4a72644f035cd335e8', 'main')
Traceback (most recent call last):
       16: from /home/kfcampbell/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/libexec/bundle:34:in `<top (required)>'
       15: from /home/kfcampbell/.rbenv/versions/2.7.3/lib/ruby/2.7.0/bundler/friendly_errors.rb:123:in `with_friendly_errors'
       14: from /home/kfcampbell/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/libexec/bundle:46:in `block in <top (required)>'
       13: from /home/kfcampbell/.rbenv/versions/2.7.3/lib/ruby/2.7.0/bundler/cli.rb:24:in `start'
       12: from /home/kfcampbell/.rbenv/versions/2.7.3/lib/ruby/2.7.0/bundler/vendor/thor/lib/thor/base.rb:476:in `start'
       11: from /home/kfcampbell/.rbenv/versions/2.7.3/lib/ruby/2.7.0/bundler/cli.rb:30:in `dispatch'
       10: from /home/kfcampbell/.rbenv/versions/2.7.3/lib/ruby/2.7.0/bundler/vendor/thor/lib/thor.rb:399:in `dispatch'
        9: from /home/kfcampbell/.rbenv/versions/2.7.3/lib/ruby/2.7.0/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
        8: from /home/kfcampbell/.rbenv/versions/2.7.3/lib/ruby/2.7.0/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
        7: from /home/kfcampbell/.rbenv/versions/2.7.3/lib/ruby/2.7.0/bundler/cli.rb:506:in `console'
        6: from /home/kfcampbell/.rbenv/versions/2.7.3/lib/ruby/2.7.0/bundler/cli/console.rb:19:in `run'
        5: from (irb):8
        4: from /home/kfcampbell/github/dev/octokit.rb/lib/octokit/client/commits.rb:201:in `compare'
        3: from /home/kfcampbell/github/dev/octokit.rb/lib/octokit/connection.rb:90:in `paginate'
        2: from /home/kfcampbell/github/dev/octokit.rb/lib/octokit/client/commits.rb:203:in `block in compare'
        1: from /home/kfcampbell/github/dev/octokit.rb/lib/octokit/client/commits.rb:203:in `concat'
TypeError (no implicit conversion of nil into Array)

Can you reproduce this error and update your branch to fix it?

[tubaxenor]

Can you reproduce this error and update your branch to fix it?

I see, I tried with curl and it does only return 300 results on page 1 and no files key on page 2, already filed a support ticket to Github and will wait for their response.

[kfcampbell]

Got it, thank you! I'm going to convert this PR to draft until we get a resolution on that issue.

[tubaxenor]

So looks like they already aware of it, and there won't be a real fix for it, the max cap is 3000 files:

[kfcampbell]

Got it. Perhaps we should close this PR until that behavior exists?

[tubaxenor]

Sure, let me do it.