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

Skip reviewer assignment on comment/review events #307

Merged
merged 1 commit into from
May 24, 2021

Conversation

bluekeyes
Copy link
Member

With the current set of predicates, these events can never change the
set of requested reviewers or trigger a re-request so there's no reason
to do the work of computing and assigning reviewers.

This is an alternate solution to the re-request loops that I fixed by
including head commit reviews in the set of reviewers. I think the other
approach is more correct, but this doesn't hurt.

The main downside is that if we add predicates that use comments or
reviews in the future, it will be easy to forget to change this trigger
condition and it might be a while before someone reports unexpected
behavior with the new predicate and reviewer assignment.

See also #306 and #302.

With the current set of predicates, these events can never change the
set of requested reviewers or trigger a re-request so there's no reason
to do the work of computing and assigning reviewers.

This is an alternate solution to the re-request loops that I fixed by
including head commit reviews in the set of reviewers. I think the other
approach is more correct, but this doesn't hurt.

The main downside is that if we add predicates that use comments or
reviews in the future, it will be easy to forget to change this trigger
condition and it might be a while before someone reports unexpected
behavior with the new predicate and reviewer assignment.
@bluekeyes bluekeyes requested a review from a team May 19, 2021 20:57
Copy link
Member

@asvoboda asvoboda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the benefit of reducing the number of Github API calls is a decent tradeoff now for a possible future bug.

@bluekeyes bluekeyes merged commit 56435f6 into develop May 24, 2021
@bluekeyes bluekeyes deleted the bkeyes/skip-reviews-on-comment branch May 24, 2021 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants