mirror of
https://github.com/tokio-rs/tokio.git
synced 2026-08-14 00:00:12 +02:00
81 lines
4.0 KiB
Markdown
81 lines
4.0 KiB
Markdown
## Reviewing Pull Requests
|
|
|
|
**Any Tokio community member is welcome to review any pull request**.
|
|
|
|
All Tokio contributors who choose to review and provide feedback on Pull
|
|
Requests have a responsibility to both the project and the individual making the
|
|
contribution. Reviews and feedback must be helpful, insightful, and geared
|
|
towards improving the contribution as opposed to simply blocking it. If there
|
|
are reasons why you feel the PR should not land, explain what those are. Do not
|
|
expect to be able to block a Pull Request from advancing simply because you say
|
|
"No" without giving an explanation. Be open to having your mind changed. Be open
|
|
to working with the contributor to make the Pull Request better.
|
|
|
|
Reviews that are dismissive or disrespectful of the contributor or any other
|
|
reviewers are strictly counter to the Code of Conduct.
|
|
|
|
When reviewing a Pull Request, the primary goals are for the codebase to improve
|
|
and for the person submitting the request to succeed. **Even if a Pull Request
|
|
does not land, the submitters should come away from the experience feeling like
|
|
their effort was not wasted or unappreciated**. Every Pull Request from a new
|
|
contributor is an opportunity to grow the community.
|
|
|
|
### Review a bit at a time
|
|
|
|
Do not overwhelm new contributors.
|
|
|
|
It is tempting to micro-optimize and make everything about relative performance,
|
|
perfect grammar, or exact style matches. Do not succumb to that temptation.
|
|
|
|
Focus first on the most significant aspects of the change:
|
|
|
|
1. Does this change make sense for Tokio?
|
|
2. Does this change make Tokio better, even if only incrementally?
|
|
3. Are there clear bugs or larger scale issues that need attending to?
|
|
4. Is the commit message readable and correct? If it contains a breaking change
|
|
is it clear enough?
|
|
|
|
Note that only **incremental** improvement is needed to land a PR. This means
|
|
that the PR does not need to be perfect, only better than the status quo. Follow
|
|
up PRs may be opened to continue iterating.
|
|
|
|
When changes are necessary, *request* them, do not *demand* them, and **do not
|
|
assume that the submitter already knows how to add a test or run a benchmark**.
|
|
|
|
Specific performance optimization techniques, coding styles and conventions
|
|
change over time. The first impression you give to a new contributor never does.
|
|
|
|
Nits (requests for small changes that are not essential) are fine, but try to
|
|
avoid stalling the Pull Request. Most nits can typically be fixed by the Tokio
|
|
Collaborator landing the Pull Request but they can also be an opportunity for
|
|
the contributor to learn a bit more about the project.
|
|
|
|
It is always good to clearly indicate nits when you comment: e.g.
|
|
`Nit: change foo() to bar(). But this is not blocking.`
|
|
|
|
If your comments were addressed but were not folded automatically after new
|
|
commits or if they proved to be mistaken, please, [hide them][hiding-a-comment]
|
|
with the appropriate reason to keep the conversation flow concise and relevant.
|
|
|
|
### Be aware of the person behind the code
|
|
|
|
Be aware that *how* you communicate requests and reviews in your feedback can
|
|
have a significant impact on the success of the Pull Request. Yes, we may land
|
|
a particular change that makes Tokio better, but the individual might just not
|
|
want to have anything to do with Tokio ever again. The goal is not just having
|
|
good code.
|
|
|
|
### Abandoned or Stalled Pull Requests
|
|
|
|
If a Pull Request appears to be abandoned or stalled, it is polite to first
|
|
check with the contributor to see if they intend to continue the work before
|
|
checking if they would mind if you took it over (especially if it just has nits
|
|
left). When doing so, it is courteous to give the original contributor credit
|
|
for the work they started (either by preserving their name and email address in
|
|
the commit log, or by using an `Author: ` meta-data tag in the commit.
|
|
|
|
_Adapted from the [Node.js contributing guide][node]_.
|
|
|
|
[node]: https://github.com/nodejs/node/blob/master/CONTRIBUTING.md
|
|
[hiding-a-comment]: https://help.github.com/articles/managing-disruptive-comments/#hiding-a-comment
|