Configurable filters for PRs to review

[ibrahima]

It would be very helpful if we could customize the filters for filters to review via the settings YAML file. Some of the default filters don't make sense for different teams, and each team will probably have their own unique set of filters that they want to use.

(original post follows)

---

Hi! First of all, thanks for this awesome project, I wanted to build something similar, but now I'm happy to not have to do so! It's actually working pretty well for our organization, we're just using the Slack notifications for now since we don't use Sendgrid.

This commit introduced a change that I don't understand: https://github.com/getsentry/probot-report/commit/8b1d4bb53d8a7961279ed307594d6b1aef1cb930

To me, it seems like you don't need review:none because if you did make a review on that PR, the PR wouldn't be returned by review-requested:${login}, because making a review dismisses the review request. Certainly when I use the Github web UI, and click on "awaiting review from you" from the dropdown, it doesn't add review:none to the query. The commit even adds a comment that implies that the behavior is a bug on Github's part, but to me it seems like expected behavior if the query includes review:none.

I'd appreciate any clarification on this matter. Thanks again!

[ibrahima]

Hmm, I guess perhaps it depends on the workflow; I also just realized that you guys exclude PRs where the owner is also an assignee on the PR, which I also don't understand. I guess this is what you meant by the fact that the app is focused on your setup. But in the case of review:none, it seems redundant to me.

The way we use the review request feature is that if someone requests changes, the author should make the changes and then request a review again, but with the review:none filter, this gets ignored. I'm curious how your workflow is that this query makes sense.

I guess for now I'll just edit my deployment and leave it at that; I think the right long-term solution is to make the filters for toReview configurable by the settings yaml file.

[jan-auer]

🎉 Thanks a lot for the feedback you provided. probot-report is still tailored to our needs at Sentry, but I would love to make it more customizable. Adding a config option for those filters is probably the best option to support different workflows.

This commit introduced a change that I don't understand: 8b1d4bb

We use team reviews at Sentry (e.g. assign the frontend or platform team). However, we don't require everyone on the team to review the PR; it's enough if one or two people do that. Unfortunately, the GitHub Search does not allow us to address this. As you said correctly, the review request is only dismissed once you review, but not someone on your team. So we chose to reduce spam and don't report the issue once someone else has reviewed.

The commit even adds a comment that implies that the behavior is a bug on Github's part, but to me it seems like expected behavior if the query includes review:none.

I don't remember all the details anymore, but the bug we mention in the comment has to do with dismissed reviews. I think that according to documentation, review:none should include issues where reviews have been dismissed after someone pushed changes, but it doesn't.

I also just realized that you guys exclude PRs where the owner is also an assignee on the PR

This was merely introduced to reduce the number of reported issues in combination with team reviews. There's no point in telling you to review your own pull request just because you're on the assigned team.

I guess perhaps it depends on the workflow

Yes, absolutely. As a matter of fact we will soon change the way we use team reviews and adapt the search queries. Still, I like your idea of making the filters configurable. 👏

[ibrahima]

Ah okay, I hadn't even realized team reviews were a thing (my company is too small to have teams of reviewers, heh). That makes a lot of sense. Anyway, it totally makes sense that the current setup is tailored to your workflow, and I'd love to contribute some day to making it more generalizable. For now, I'm fine just editing the filters on my local installation, it already works pretty well for me as-is.