Skip to content

Commit

Permalink
Merge pull request ethereum#13744 from ethereum/review-checklist-upda…
Browse files Browse the repository at this point in the history
…te-for-external-prs

Update the review checklist to address common external PR problems
  • Loading branch information
cameel authored Nov 30, 2022
2 parents 68686b5 + fdf4ec5 commit c6ee18a
Showing 1 changed file with 26 additions and 0 deletions.
26 changes: 26 additions & 0 deletions ReviewChecklist.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ and solve common issues before they are pointed out in the review.
It is also meant to serve as a final checklist for reviewers to go through before approving a PR.

## Before You Submit a PR
- [ ] **Do you have any other open PRs?**
Work on a PR is not done until it is merged or closed.
Our reviewing capacity is limited, so we require that external contributors work on **no more than one PR at a time**.
- If your PR is not getting reviewed, feel free to bring it to our attention on the [#solidity-dev](https://gitter.im/ethereum/solidity-dev) channel.
- Unless they were requested, we are going to close any excess PRs, leaving only the earliest one open.
You may reopen them, one at a time, when your current PR is done.
- [ ] **Is the issue ready to be worked on?**
- If the issue does not have a desirability label (`nice to have`, `should have`,
`must have eventually`, `must have`, `roadmap`) we have not yet decided whether to implement it.
Expand Down Expand Up @@ -34,6 +40,26 @@ It is also meant to serve as a final checklist for reviewers to go through befor
- Review updated files before committing them.
**Are expectations correct and do updated tests still serve their purpose?**

## Abandoned PRs
- [ ] **Is the submitter still responsive?**
- If the PR had no activity from the submitter in the last 2 weeks (despite receiving reviews and our prompts) we consider it abandoned.
- [ ] **Is the abandoned PR easy to finish or relevant?**
- Apply the `takeover` label if the PR can be finished without significant effort or is something that actually needs to be done right now.
Otherwise close it.
It can still be taken over later or reopened by the submitter but until then we should not be getting sidetracked by it.

## Light Review
Before an in-depth review, it is recommended to give new PRs a quick, superficial review, which
is not meant to provide complete and detailed feedback, but instead give the submitter a rough idea
if the PR is even on the right track and let them solve the obvious problems on their own.

Light review should focus on the following three areas:
- [ ] **Are there any obvious mistakes?** Style issues, bad practices, easy to identify bugs, etc.
- [ ] **Is there anything missing?** Tests (of the right kind), documentation, etc. Does it address the whole issue?
- [ ] **Is it the right solution?** Are there better ways to do this? Is the change even necessary?

If the answers above are "Yes, Yes, No", thank the contributor for their effort and **close the PR**.

## Coding Style and Good Practices
- [ ] Does the PR follow our [coding style](CODING_STYLE.md)?

Expand Down

0 comments on commit c6ee18a

Please sign in to comment.