danger / danger-js

⚠️ Stop saying "you forgot to …" in code review
http://danger.systems/js/
MIT License
5.24k stars 367 forks source link

[BUG] danger.github.reviews only providing the first 30 reviews #1383

Open jcook-uptycs opened 1 year ago

jcook-uptycs commented 1 year ago

Describe the bug A clear and concise description of what the bug is.

When using danger.github.reviews it only includes the first 30 reviews on the PR.

To Reproduce Steps to reproduce the behavior:

  1. Have a PR with more than 30 reviews.
  2. Use
    console.log(danger.github.reviews.length);

Expected behavior

Expect to have the correct number of reviews.

Screenshots If applicable, add screenshots to help explain your problem.

Your Environment

software version
danger.js 11.2.6
node v16.20.0
npm 8.19.4
Operating System Linux and MacOS

Additional context Add any other context about the problem here.

jcook-uptycs commented 1 year ago

It looks like 30 is the default provided by the GitHub API. https://docs.github.com/en/rest/pulls/reviews?apiVersion=2022-11-28#list-reviews-for-a-pull-request

It can go up to 100.

jcook-uptycs commented 1 year ago

Can danger be updated to get the max 100, or handle paging if more than 30?

orta commented 1 year ago

Sounds like an impressive PR, but yeah, I think either of those options (auto-paginating, or bumping the number to be higher) is totally fine

fbartho commented 1 year ago

I’d be happy to review a PR if you want to submit a patch @jcook-uptycs

We have the whole Github API Client embedded, so you should be able to trace it to either increase the limit, or set something up for auto-pagination.

fbartho commented 1 year ago

Depending on what you’re doing, would ensuring the reviews are fetched by increasing age solve your problem?

Or are you hunting for specific reviewers or something? — Github has an Owners pattern, so if you want to require reviews from specific folks, you don’t need DangerJS

jcook-uptycs commented 1 year ago

Depending on what you’re doing, would ensuring the reviews are fetched by increasing age solve your problem?

Or are you hunting for specific reviewers or something? — Github has an Owners pattern, so if you want to require reviews from specific folks, you don’t need DangerJS

I am looking for reviews from certain people, but they are not code owners. It's a secondary approval. Example after the code owners have approved then someone in QA has to approve.

@fbartho If you are not talking about CODEOWNERS, can you include a link to info on it?

fbartho commented 1 year ago

No @jcook-uptycs, I was indeed talking about CODEOWNERS -- I just wanted to make sure there wasn't a silly option you hadn't considered!

Based on your use-case it does sound like you want to expand the reviews you're checking for! -- That said, you could solve this by using the danger.github raw API client directly if you don't want to patch DangerJS!