feat: Add PR requester for external reviewers

[CPerezz]

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.

[CPerezz]

In order to make this work, we need that all the added participants of the external-reviewers group declared in .github/reviewer-lottery.yml

To prove this works I made the PR now just with @icemelon as is the only of the members who is added as collaborator for this repo. So it works.

[icemelon]

@CPerezz Yes, I received the email notification of the review request to this PR.

[ChihChengLiang]

@DreamWuGit can you check your email for repo invitation?

[DreamWuGit]

@DreamWuGit can you check your email for repo invitation?
just check zkevm-circuit repo invitation and accepted :) Do you mean review request email for this PR ?

[ChihChengLiang]

@DreamWuGit Thank you :) I mean just the repo invitation, not the review request.

[CPerezz]

@ChihChengLiang @icemelon could you take another look and approve? All the members are set now 🎉

[ChihChengLiang]

LGTM

[ntampakas]

Moving conversation from #467

Following up on the previous message, here is the implementation we have concluded on with pros and cons.

1. Every PR (either from fork or not) creates and uploads an artifact on base repo, consisting of all PR info.
2. A second workflow runs periodically as a scheduled task, downloads and extracts the artifact and in general may run any action requiring access to GitHub token: In our case the action is to randomly select reviewer(s) and assign them in PR.
   Pros:
   No need to distribute tokens to external accounts, as first artifact creation requires no write permission.
   Same concept can be used for providing benchmark results in comments.

Cons:
Second workflow is executed as a cron job, so it will flood the actions history with workflow run records. It will make it a bit annoying to read the actions history (without filtering).
As mentioned in the comment above, Github does no guarantee that the workflow using the schedule event, will run at the pre-defined time. Therefore reviewers assignment, might take more than expected.

If everyone agrees on this approach, i will shortly raise a PR for review.

[ntampakas]

@DreamWuGit,
Regarding the last GH Action that worked: Did you use a new token or did you try something different?

This probably needs further investigation.

@ChihChengLiang , did you by any chance apply different permission levels against @DreamWuGit account/repo?
Just trying to figure out what is wrong :)

[CPerezz]

Moving conversation from #467

Following up on the previous message, here is the implementation we have concluded on with pros and cons.

1. Every PR (either from fork or not) creates and uploads an artifact on base repo, consisting of all PR info.

2. A second workflow runs periodically as a scheduled task, downloads and extracts the artifact and in general may run any action requiring access to GitHub token: In our case the action is to randomly select reviewer(s) and assign them in PR.
   Pros:
   No need to distribute tokens to external accounts, as first artifact creation requires no write permission.
   Same concept can be used for providing benchmark results in comments.

Cons: Second workflow is executed as a cron job, so it will flood the actions history with workflow run records. It will make it a bit annoying to read the actions history (without filtering). As mentioned in the comment above, Github does no guarantee that the workflow using the schedule event, will run at the pre-defined time. Therefore reviewers assignment, might take more than expected.

If everyone agrees on this approach, i will shortly raise a PR for review.

The solution doesn't look too resilient. Seems that introduced overhead and also we need to wait for the assignations anyway.
My suggestion is to simply remove this workflow for now and doo the assignation of the Scroll members manually.

[ntampakas]

Moving conversation from #467
Following up on the previous message, here is the implementation we have concluded on with pros and cons.

1. Every PR (either from fork or not) creates and uploads an artifact on base repo, consisting of all PR info.

2. A second workflow runs periodically as a scheduled task, downloads and extracts the artifact and in general may run any action requiring access to GitHub token: In our case the action is to randomly select reviewer(s) and assign them in PR.
   Pros:
   No need to distribute tokens to external accounts, as first artifact creation requires no write permission.
   Same concept can be used for providing benchmark results in comments.

Cons: Second workflow is executed as a cron job, so it will flood the actions history with workflow run records. It will make it a bit annoying to read the actions history (without filtering). As mentioned in the comment above, Github does no guarantee that the workflow using the schedule event, will run at the pre-defined time. Therefore reviewers assignment, might take more than expected.
If everyone agrees on this approach, i will shortly raise a PR for review.

The solution doesn't look too resilient. Seems that introduced overhead and also we need to wait for the assignations anyway. My suggestion is to simply remove this workflow for now and doo the assignation of the Scroll members manually.

Indeed, it adds unnecessary complexity.
Please let us investigate it a little bit more, since DreamWuGit` s last PR worked with that GH action.

[DreamWuGit]

@DreamWuGit, Regarding the last GH Action that worked: Did you use a new token or did you try something different?

This probably needs further investigation.

@ntampakas you mean github personal token in setting ? I don't change it as it is not expired at the moment , I am not aware of something different rather than git push operation ! feel free to let me know if you need to check specific

[0xmountaintop]

This problem can be simply solved by changing "pull_request"(external_reviewers_assig.yml#L3) to "pull_request_target"

As explained at the end of https://github.com/marketplace/actions/auto-request-review

Note that with the recent change to GitHub Actions that are created by Dependabot, the pull_request event will no longer give access to your secrets to this action. Instead you will need to use the pull_request_target event. If you do this make sure to read Keeping your GitHub Actions and workflows secure: Preventing pwn requests to understand the risks involved.

& at the end of https://github.com/marketplace/actions/reviewer-lottery

Why on pull_request_target?

By running this action on pull_request_target we enable this action to be performed on PRs opened by users with readonly access to the repo, for example those by Dependabot.

[ntampakas]

@HAOYUatHZ , this really seems to resolve the issue (verifying after testing against local GH accounts).
What concerns me is, why sometimes the action works and sometimes not.

@CPerezz , may i ask what was the reason this workflow was implemented using the pull_request event and not pull_request_target?

[ntampakas]

Also, as far as i can see from succeeded Reviewer lottery GH actions, external-zkevm-reviewers from reviewer-lottery.yml file seem not to be assigned in any PR coming from fork.

[CPerezz]

@HAOYUatHZ , this really seems to resolve the issue (verifying after testing against local GH accounts). What concerns me is, why sometimes the action works and sometimes not.

@CPerezz , may i ask what was the reason this workflow was implemented using the pull_request event and not pull_request_target?

No particular reason. We can change it but I don't think is going to make any difference.

Also, as far as i can see from succeeded Reviewer lottery GH actions, external-zkevm-reviewers from reviewer-lottery.yml file seem not to be assigned in any PR coming from fork.

This is because they're requested a review from the CODEOWNERS file directly.