CI: Upgrade EIP-Bot to eip-review-bot #5508
Conversation
|
Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s): (fail) .github/workflows/auto-review-bot.yml
(fail) config/eip-editors.yml
|
Pandapip1
added
waiting: editor to review
e-review
Pandapip1
changed the title
|
There has been no activity on this pull request for 2 weeks. It will be closed after 3 months of inactivity. If you would like to move this PR forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review. |
|
This would be useful to have. |
Pandapip1
added
e-consensus
| - name: Auto Review Bot | ||
| id: auto-review-bot | ||
| uses: ethereum/EIP-Bot@b81356bd8302a99e2d2bcf2bb5d2d983a12f7b8d | ||
| uses: Pandapip1/eip-review-bot@dist |
There was a problem hiding this comment.
I love this change. Just a nitpick: would you move it to the ethereum or etheruem-cat-herders org so it can be a community repo instead of a personal repo?
There was a problem hiding this comment.
Couldn't agree more. I can't do that without the help of someone with the permission to create repos in the ethereum org (a perm that I don't have), however.
There was a problem hiding this comment.
How about putting it into the ECH? I think pooja probably have permission.
There was a problem hiding this comment.
I would be okay with that @poojaranjan
There was a problem hiding this comment.
Is there a reason to put in ECH vs. ethereum? All previous tools have been moved to ethereum org, I think it's good to keep that pattern.
There was a problem hiding this comment.
I agree - but they can always also then be moved to ethereum afterward. I'll ask if @poojaranjan has create repo perms on @ethereum.
Co-authored-by: lightclient <[email protected]>
|
@Pandapip1 can you rebase pls. @SamWilsn - thoughts migrating to eip-review-bot? |
|
I'd like to leave some feedback on the bot itself before we make the switch, but besides that, I have no problem with this! |
|
I have left some feedback on eip-review-bot. Code is very understandable! My only concern is that there aren't any automated tests that I could see. Personally I'd like to see some automated tests before we switch away from eip-bot. What do you think @lightclient? |
|
I believe my concerns regarding automated testing have mostly been resolved. I am still nervous about switching to a new merge guardian, and would like to see some tests that exercise the whole process (including mocking the GitHub API.) That said, I think this is sufficiently tested to give it a trial run. Shall we bring it up at the next EIPIP meeting? |
|
Sure thing. I might be able to attend the next EIPIP meeting. |
|
Shoot, should've brought this up on EIPIP today. I'm still interested in either getting this merged, or fixing the file deletion bug in the current bot. |
|
@SamWilsn we're merging this, right? |
* Use eipm * EIP editors config file * Remove unneccesary checkout action * Remove setup nodejs env * New name * Remove lightclient as ERC editor Co-authored-by: lightclient <[email protected]> * Fix minor change --------- Co-authored-by: lightclient <[email protected]>
Has a few advantages over EIP-Bot:
Progress: