Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some tasks are being skipped (pagination needed for +100 pr comments) #19

Open
billsacks opened this issue Apr 16, 2021 · 6 comments
Open
Labels
enhancement New feature or request

Comments

@billsacks
Copy link

I have been using this app for about 8 months now and it has been working great – thank you!

Today, for the first time, I noticed that it is skipping (not counting) some tasks in one of our PRs: ESCOMP/CTSM#1301. Two check boxes that seem to be skipped are the ones in ESCOMP/CTSM#1301 (comment) and ESCOMP/CTSM#1301 (comment) – checking and unchecking those has no effect on the summary count, even though others in the PR (like ESCOMP/CTSM#1301 (review)) are counted. There seem to be a few others in the PR that also aren't counted.

I can't tell what's going on here: I tried making a new PR in a test repo to reproduce this problem and wasn't able to.

This isn't critical for me, since I tend to double-check the final count with my own tool (https://github.com/ESMCI/github-tools), so feel free to close as a wontfix if you also can't reproduce it – but I wanted to raise this in case it's a more general issue.

@stilliard
Copy link
Owner

stilliard commented Apr 16, 2021

Hey @billsacks !
Thanks, glad you're getting good use of the app still.

Ah interesting, I wonder if it's an issue where maybe GitHub's only giving me a limited set of comments or similar? I'll check with a test repo and see if after a certain number of comments maybe it stops.

Thanks for reporting the issue, will keep you posted as I debug.

@stilliard
Copy link
Owner

Interesting, in a test it stopped at 30 comments:
#20

Will be back online over the weekend so I'll check the API docs then as maybe GitHub has a default limit of 30 per page?
Hopefully we can just bump that to 100 or more, or we could easily add in paging over the comments if needed.

@stilliard
Copy link
Owner

In fact, I've got a couple minutes before I need to disappear tonight.
Found the limit for comments on PR's is in fact 30, ref: https://docs.github.com/en/rest/guides/traversing-with-pagination

I'll bump this with the per_page param to 100 for now but will also look to come back and add full paging support in too.

@billsacks
Copy link
Author

Awesome, thanks a lot for looking into this!

@stilliard
Copy link
Owner

No problem, thanks for finding the bug!

The fix for checking up to 100 of each type of comment on a PR is now merged and live, when you can could you let me know if that's resolved the issue you were seeing too or if there's still any issue, thanks.

I'll keep the issue open for now to remind me to come back and add proper pagination in as well.

Hope you have a great weekend 👋

@billsacks
Copy link
Author

Yes, the PR that had issues before now has a correct count – thanks again!

(We do sometimes have PRs that accumulate hundreds of comments, so long-term it would be awesome to have this pagination, but that's not at all critical.)

@stilliard stilliard changed the title Some tasks are being skipped Some tasks are being skipped (pagination needed for +100 pr comments) Oct 19, 2023
@stilliard stilliard added the enhancement New feature or request label Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants