Ignore team reviews #37
Conversation
Updating dependencies was required because our package still contained the corrupted event-stream package, and it was causing the deployment to fail.
The GraphQL request didn't return all of the informations required to distinguish users & teams because of OAuth App access restrictions on the Angular repository. I switched back to rest requests which now contain all of the information necessary (that was not the case a few months ago). We now only count the user request reviews instead of users + teams.
ocombe
requested review from
IgorMinar and
alexeagle
and removed request for
alexeagle
|
|
||
| return sizeArtifacts.firestore.runTransaction(async transaction => { | ||
| const results = await transaction.getAll(...artifactDocs); | ||
| const results = await transaction.getAll(...artifactDocs as [any]); |
There was a problem hiding this comment.
I had to force this type to any, because Typescript throws an error saying that it "Expected at least 1 arguments, but got 0 or more.". Apparently it doesn't handle length on spread arrays very well.
The previous version of probot was triggering deprecated error messages in the logs because it was using old @octokit/rest functions. The new version has been updated to use the new package. A lot of function names/types have changed because of it.
IgorMinar
left a comment
IgorMinar
left a comment
There was a problem hiding this comment.
also consider documenting the spec for this check in an md file in this repo, including why we include or exclude certain types of reviews. otherwise we won't remember in the future..
the rest looks good (we discussed in person that adding a test for this is prohibitively difficult). please clean up.
| // given the review yet. Once he does, he disappears from this list, and we need to check the reviews | ||
| const reviewRequests = prData.reviewRequests.nodes.length; | ||
| const usersReviews: string[] = []; | ||
| const reviewRequests =(await context.github.pullRequests.listReviewRequests({owner, repo, number: pr.number})).data.users; |
There was a problem hiding this comment.
please write the code above in a gender neutral way: "Once they do...". And please check for similar mistakes throughout the repo, I've seen you make this mistake before so I suspect that there are more issues. Let's be a bit more welcoming and inclusive to all the people reading the code.
There was a problem hiding this comment.
I saw this comment yesterday when I was updating the code and thought that it was badly phrased but I forgot to update it, thanks for the reminder.
I'll look for other similar comments.
| // given the review yet. Once he does, he disappears from this list, and we need to check the reviews | ||
| const reviewRequests = prData.reviewRequests.nodes.length; | ||
| const usersReviews: string[] = []; | ||
| const reviewRequests =(await context.github.pullRequests.listReviewRequests({owner, repo, number: pr.number})).data.users; |
There was a problem hiding this comment.
also please document that this is where we filter out group reviews and consider only reviews from individuals..
ocombe
force-pushed
the
ignore-team-reviews
branch
from
940e254 to
a5cf05a
Compare
|
PR updated |
3 participants
We now only count the user request reviews instead of users + teams.
I also added a filter to ignore reviews from people who aren't members of the repository.
Updating dependencies was required because our package still contained the corrupted event-stream package, and it was causing the deployment to fail.
The GraphQL request didn't return all of the informations required to distinguish users & teams because of OAuth App access restrictions on the Angular repository.
I switched back to rest requests which now contain all of the information necessary (that was not the case a few months ago).