showmyprs.com icon indicating copy to clipboard operation
showmyprs.com copied to clipboard

limit of 600 PR ?

Open pborreli opened this issue 7 years ago • 11 comments

https://showmyprs.com/user/pborreli indicates Total pull requests sent: 600

It should be ~1204 instead

And it says : Total contributed repositories: 459

It should be >800

pborreli avatar Mar 02 '17 12:03 pborreli

https://showmyprs.com/user/shurcooL says 722 PRs, so that means 600 cannot be an explicit limit. There may be another issue causing not all PRs to be displayed though.

Where did you get the 1204 number from?

dmitshur avatar Mar 02 '17 19:03 dmitshur

from https://github.com/pulls

image

pborreli avatar Mar 02 '17 21:03 pborreli

Got it. For me, it lines up pretty well. GitHub shows 7 open + 718 closed PRs created, which adds up to 725. showmyprs gives me 722, so there is a discrepancy of 3 PRs that I can't explain.

image

dmitshur avatar Mar 02 '17 21:03 dmitshur

Here's an idea, are any of your PRs made in private repositories? showmyprs wouldn't be able to show those, since it uses public data only.

dmitshur avatar Mar 02 '17 21:03 dmitshur

Yeah, I think that's exactly it. If I select "Public repositories only" under Repository visibility" filter, then GitHub shows me exactly 722 PRs in total (3 are in private repos), which matches my showmyprs count perfectly.

image

Can you try the same and see what you get?

dmitshur avatar Mar 02 '17 21:03 dmitshur

I have only 4 PR in private repos

image

pborreli avatar Mar 02 '17 21:03 pborreli

I see, then there's definitely some sort of issue in showmyprs.

I can confirm that https://github.com/search?q=type%3Apr+author%3Apborreli+is%3Apublic&type=Issues finds 1210 public PRs for your username:

image

I'll leave this to @karanjthakkar to investigate further.

It seems the issue should be somewhere in this code:

https://github.com/karanjthakkar/showmyprs.com/blob/a651f2654cb3afcd384d20cb1aa89279188df740/main.go#L87-L181

However, I'm not spotting any issues there just by looking at it. TotalPrs is set to len(allPrs), and allPrs should correctly contain all PRs (I don't see any reasons why it wouldn't).

It's also possible some network connectivity issue caused the for loop to break out early (because NextPage would be 0 if there was an error getting an actual response).

dmitshur avatar Mar 02 '17 21:03 dmitshur

thanks @shurcooL for the help 💯👍

pborreli avatar Mar 02 '17 21:03 pborreli

@pborreli Sorry for getting back to you so late about this. I have been travelling thee past few days.

  1. The probable reason why the first time you were shown a total of only 600, was because I'm using my personal access token for Github PR search and it has a limit of 30 for a window of 15 minutes with a max per page results of 100. So it is possible that when I started fetching list of PR's for your username, I had already consumed 24 searches for that window.

  2. Having refreshed your profile on my end, I found that it is now showing a total of 1000 PR's sent. This is a limit on Github's end. Since you have a total of 1200 something PR's I have to search 13 times with different page numbers. On the 11th page, Github says: Only the first 1000 search results are available

In case 1, I am planning to fix by giving the user an option to login with their account so that I can use their access token for it. This should fix the incomplete PR count issue.

In case 2, however, there is no way for me to bypass the 1000 search results hard limit set by Github. I will investigate more on whether there are other ways to fetch your PR's without querying the search API.

Sorry if I can't fix this sooner but I hope this gives you more clarity on what the issue is.

cc: @shurcooL Just FYI :) Thanks for chipping in before and helping investigate this with @pborreli 💯

karanjthakkar avatar Mar 05 '17 05:03 karanjthakkar

wow thanks for the investigation,

I would log without problem is the scope is readonly and public,

Maybe the solution without the need of login would be the search the api looping on year ? like in this answer ?

pborreli avatar Mar 06 '17 09:03 pborreli

@pborreli Thanks for searching for a workaround this issue. I think this is doable. I might need to refactor some logic of recursively fetching newer search pages. I'll add this to my list of todos. Alternatively, if you would like to take a shot at a PR for this, I'd be more than happy to help you out with that!

karanjthakkar avatar Mar 09 '17 08:03 karanjthakkar