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.

Sign up for GitHub

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

Jump to bottom

Force pushing in a PR causes the workflow to pick a new reviewer from a team #26

Closed
CPerezz opened this issue Apr 12, 2022 · 2 comments
Closed

Force pushing in a PR causes the workflow to pick a new reviewer from a team #26

CPerezz opened this issue Apr 12, 2022 · 2 comments

Comments

@CPerezz
Copy link

CPerezz commented Apr 12, 2022

I think it should be mentioned somewhere that if the workflow is configured like:

on:
  pull_request:
    types: [synchronize, opened, reopened, ready_for_review]

The workflow internal logic is not prepared to re-select the same reviewer from a team and instead will assign a new one. Leading into PRs with multiple reviewers (depending on the amount of rebases and force-pushes done).

Should this be something that has to be avoided and we should config the workflow only like:

on:
  pull_request:
    types: [opened]

Or should instead the logic of the tool be reworked when it comes to teams?
CPerezz added a commit to privacy-scaling-explorations/zkevm-circuits that referenced this issue Apr 12, 2022
@CPerezz
Trigger workflow only when PR is opened
872de15 
Force-pushing to the PR currently re-triggers the workflow by assigning
to the PR a new member of the team while mantaining the previous.
That's an issue as we end up with multiple reviewers on each PR when
indeed we just need one.

See: uesteibar/reviewer-lottery#26 for more
details.
CPerezz added a commit to privacy-scaling-explorations/zkevm-circuits that referenced this issue Apr 12, 2022
@CPerezz
Trigger workflow only when PR is opened
8d28c15 
Force-pushing to the PR currently re-triggers the workflow by assigning
to the PR a new member of the team while mantaining the previous.
That's an issue as we end up with multiple reviewers on each PR when
indeed we just need one.

See: uesteibar/reviewer-lottery#26 for more
details.
CPerezz added a commit to privacy-scaling-explorations/zkevm-circuits that referenced this issue Apr 13, 2022
@CPerezz
feat: Add PR requester for external reviewers (#446)
c12d61f 
* feat: Add PR requester for external reviewers

Deprecates #438 and instead adds the external_reviewers to the PR
reviews using a 3rd party application.

That is done like this to avoid including a lot of people into
`appliedzkp` org and granting custom permissions each time we want to
add a new reviewer.

Resolves: #429 completely.

* Trigger workflow only when PR is opened

Force-pushing to the PR currently re-triggers the workflow by assigning
to the PR a new member of the team while mantaining the previous.
That's an issue as we end up with multiple reviewers on each PR when
indeed we just need one.

See: uesteibar/reviewer-lottery#26 for more
details.
@uesteibar
Copy link
Owner

@CPerezz the readme recommend the following cases actually: [opened, ready_for_review, reopened]. Indeed if you use synchronize you'll get re-assigns when you push force.

Actually using only opened will not work for every case, because the tool is write as to not to assign any reviewer for PRs that are in draft state (thus the need for ready_for_review)

@CPerezz
Copy link
Author

CPerezz commented May 11, 2022

Thanks a lot! I missed the README part!!! :(

@CPerezz CPerezz closed this as completed May 11, 2022
omega000133 added a commit to omega000133/zkevm-circuits that referenced this issue May 17, 2024
@omega000133
feat: Add PR requester for external reviewers (#446)
f990627 
* feat: Add PR requester for external reviewers

Deprecates #438 and instead adds the external_reviewers to the PR
reviews using a 3rd party application.

That is done like this to avoid including a lot of people into
`appliedzkp` org and granting custom permissions each time we want to
add a new reviewer.

Resolves: #429 completely.

* Trigger workflow only when PR is opened

Force-pushing to the PR currently re-triggers the workflow by assigning
to the PR a new member of the team while mantaining the previous.
That's an issue as we end up with multiple reviewers on each PR when
indeed we just need one.

See: uesteibar/reviewer-lottery#26 for more
details.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@uesteibar @CPerezz