EIPs icon indicating copy to clipboard operation
EIPs copied to clipboard

Stale review dismissal loop

Open lightclient opened this issue 3 years ago • 3 comments

Pull Request

https://github.com/ethereum/EIPs/pull/5793

We need to clean up the PR output so there aren't a ton of stale review dismissals.

Screen Shot 2022-11-02 at 07 56 29

Relevant log output

No response

lightclient avatar Nov 02 '22 13:11 lightclient

Not possible; this is a GitHub limitation ("feature").

Pandapip1 avatar Nov 02 '22 15:11 Pandapip1

How is it a github limitation? How do other systems deal with this?

lightclient avatar Nov 02 '22 19:11 lightclient

How do other systems deal with this?

They don't have reviews automatically dismissed on push. This is required for the EIPs repository, however, since PRs auto-merge.

Pandapip1 avatar Nov 04 '22 00:11 Pandapip1

Hi.

Could ? we try this:

Settings ▶ Branches ▶ Branch protection rules ▶ Edit ▶ Uncheck "Dismiss stale pull request approvals when new commits are pushed".

Either way I will take a look into EIP-Bot to see if there is a way to do it from the Bot.

JEAlfonsoP avatar Nov 18 '22 15:11 JEAlfonsoP

No, we can't do that. That permission must be checked to ensure that reviews are dismissed when new commits are pushed to branches, as you can see in the screenshot @lightclient linked.

Pandapip1 avatar Nov 18 '22 17:11 Pandapip1

So, you want to dismiss but not being notify ( no echo ) ?

JEAlfonsoP avatar Nov 18 '22 20:11 JEAlfonsoP

I think that is the goal, yes.

Pandapip1 avatar Nov 18 '22 22:11 Pandapip1

Hi @Pandapip1 , I would like to explore two possible options to silent this alert (or any other one), 1st Have you tried/used Code Scanning (workflow) filters for the repo ? and 2nd There is a REST API for alerts/{alert_number} endpoint, have you tried/used it ? Sorry for the direct questions, I just do not want to expend time in something that you may be already has tried it.. Ty.

JEAlfonsoP avatar Nov 19 '22 19:11 JEAlfonsoP

Forgive me for the probably naive question, but how would enabling code scanning fix this problem?

Pandapip1 avatar Nov 19 '22 19:11 Pandapip1

Thanks for answering ! I would see if this (or any other alert) is listed in code scanning alerts and from there would use the -tag: filter to silent it. But, as I said if you have not worked in it I will stop writing assumptions and will try to test it: implementing the code scanning workflow or coding the API into EIP-Bot. If you have any other idea please share it so I can either study it.

JEAlfonsoP avatar Nov 19 '22 21:11 JEAlfonsoP

This is unrelated to EIP bot; this is a GitHub feature (logging automatic review dismissals in the timeline) that we want to disable.

Pandapip1 avatar Nov 19 '22 23:11 Pandapip1

I believe that either, but I would like to find out each workflow's signal and see if we can silent (in case that is desired) it or not, one is what you mentioned above generated by GitHub itself. If you agree I can proceed with the testing and If you are working on it, again I kindly ask you to let me know, and in case that you want to coordinate this study, let me know so I can DM you and so all the work goes in the same direction. Ty.

JEAlfonsoP avatar Nov 20 '22 14:11 JEAlfonsoP

Feel free to do testing. It is almost certainly caused by the dismiss stale reviews branch protection option, however.

Pandapip1 avatar Nov 20 '22 17:11 Pandapip1

Ty. I will do

JEAlfonsoP avatar Nov 20 '22 20:11 JEAlfonsoP

There has been no activity on this issue for 1 week. It will be closed after 3 months of inactivity.

github-actions[bot] avatar Jan 15 '23 00:01 github-actions[bot]

I think this was a GitHub bug. The PR in question has been closed.

Pandapip1 avatar Jan 15 '23 13:01 Pandapip1