Difference between revisions of "Code review"
m (→Good Reads) |
(first stab at guidelines) |
||
| Line 1: | Line 1: | ||
This page documents code review practices used for [[Software Heritage]] development. | This page documents code review practices used for [[Software Heritage]] development. | ||
| − | + | '''WORK IN PROGRESS page''' | |
| − | + | == Guidelines == | |
| + | |||
| + | # Code reviews (CRs) are '''strongly recommended''' for any code change, but not mandatory (nor enforced at the VCS level). | ||
| + | # The CR [[Code review in Phabricator|'''workflow''']] is implemented using Phabricator/Differential. | ||
| + | # '''Suggest reviewer'''(s) when submitting new CR requests: either the most knowledgeable person(s) for the target code or the general [https://forge.softwareheritage.org/project/view/50/ Reviewers] group. | ||
| + | # '''Review anything you want''': no matter the suggested reviewer(s), feel free to review any outstanding CR. | ||
| + | # '''One LGTM is enough''': feel free to approve any outstanding CR. | ||
| + | # '''Review every day''': CRs should be timely as fellow developers will wait for them. To make CRs team-sustainable each developer should allocate a fixed ''minimum'' amount of time for doing CR every (work ☺) day. | ||
| + | |||
| + | For more detailed suggestions (and much more) on the motivational and practical aspects of code reviews see Good reads below. | ||
== Good reads == | == Good reads == | ||
| Line 14: | Line 23: | ||
* [https://blog.codinghorror.com/code-reviews-just-do-it/ Motivation: code quality] (Coding Horror) | * [https://blog.codinghorror.com/code-reviews-just-do-it/ Motivation: code quality] (Coding Horror) | ||
* [http://www.processimpact.com/articles/humanizing_reviews.pdf Motivation: humanizing peer reviews] (Wiegers) | * [http://www.processimpact.com/articles/humanizing_reviews.pdf Motivation: humanizing peer reviews] (Wiegers) | ||
| + | |||
| + | == See also == | ||
| + | |||
| + | * [[Code review in Phabricator]] | ||
[[Category:Software development]] | [[Category:Software development]] | ||
Revision as of 12:34, 12 October 2018
This page documents code review practices used for Software Heritage development.
WORK IN PROGRESS page
Guidelines
- Code reviews (CRs) are strongly recommended for any code change, but not mandatory (nor enforced at the VCS level).
- The CR workflow is implemented using Phabricator/Differential.
- Suggest reviewer(s) when submitting new CR requests: either the most knowledgeable person(s) for the target code or the general Reviewers group.
- Review anything you want: no matter the suggested reviewer(s), feel free to review any outstanding CR.
- One LGTM is enough: feel free to approve any outstanding CR.
- Review every day: CRs should be timely as fellow developers will wait for them. To make CRs team-sustainable each developer should allocate a fixed minimum amount of time for doing CR every (work ☺) day.
For more detailed suggestions (and much more) on the motivational and practical aspects of code reviews see Good reads below.
Good reads
Good reads on different angles of code review:
- Best practices (Palantir) ← comprehensive and recommended read, especially if you're short on time
- Review checklist (Code Project)
- Motivation: team culture (Google & FullStory)
- Motivation: code quality (Coding Horror)
- Motivation: humanizing peer reviews (Wiegers)
