search

node-gh / gh

(DEPRECATED) GitHub CLI made with NodeJS. Use the official https://cli.github.com/ instead.
http://nodegh.io
Other
1.71k stars 217 forks source link

Add pagination #542

Closed m-roberts closed 6 years ago

m-roberts commented 6 years ago

This does this for repo functions. I imagine that this can be easily updated to work for more situations. Aims to resolve #539

I accept that this might need extending, but I am not sure how best to do that.

protoEvangelion commented 6 years ago

@m-roberts Thanks for the PR. I will look into this now.

protoEvangelion commented 6 years ago

Practically users will not notice any difference on their end, this is just to work with the Github API better is that correct?

And when you say "This does this for repo functions" do you mean it will apply only to gh repo commands for now?

Also, when it comes to extending, do you envision being able to do something like --limit 50 or --all on each command?

m-roberts commented 6 years ago

So at the moment, there is a lot of per_page: 1000 in the code, when Github actually limits it to 100 per page.

I wanted to list all of the repositories in my Github organisation (which is current at over 200), and so it was necessary to extend the functionality to go beyond only returning the first 100.

The user would not notice any difference other than ensuring that they get a comprehensive list when requesting as such.

This suited my use case, but I am not comfortable with the entire codebase to know how to extend this to ensure that, where the number of results is over 100 (e.g. PRs, notifications, issues, gists), that all results are returned. This might not even be a problem, but I regrettably do not have the time to set up the use cases to test this.

I wanted to contribute what I had achieved, in the hopes that this can be extended to handle any other use cases which are not handling pagination.

Perhaps we should start by updating the per_page: 1000s to be 100, so that it is less misleading?

protoEvangelion commented 6 years ago

Thanks for clarifying 😄

It looks like it is working great. However, is it printing out all of your org's repos?

You can log out the number like this:

Pull request submitted to https://github.com/m-roberts/gh/pull/1. See changes here.

My org has around 297 repos:

image

But it is only listing 263. Is something similar happening on your end?

protoEvangelion commented 6 years ago

If this works correctly for just the repo command, I will go ahead and release this and explore extending this to the other commands.

I do like the idea of at least changing the '1000' limit to '100' so it is more clear.

Thanks for the great work!

m-roberts commented 6 years ago

I have just made a couple of changes - for me, I was actually getting some duplicates.

Now, gh repo --list --org google | wc -l for me returns 1303, which at time of writing, is what I am observing here. I checked the resulting response for duplicates, of which I found none. This leads me to believe that it is working correctly!

protoEvangelion commented 6 years ago

Great work! I resolved the test errors for Node v4 & merged. Thanks again 😄