Difference between revisions of "Code review"

From Software Heritage Wiki
Jump to navigation Jump to search
(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.
  
== Implementation ==
+
'''WORK IN PROGRESS page'''
  
See [[Code review in Phabricator]].
+
== 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

  1. Code reviews (CRs) are strongly recommended for any code change, but not mandatory (nor enforced at the VCS level).
  2. The CR workflow is implemented using Phabricator/Differential.
  3. Suggest reviewer(s) when submitting new CR requests: either the most knowledgeable person(s) for the target code or the general Reviewers group.
  4. Review anything you want: no matter the suggested reviewer(s), feel free to review any outstanding CR.
  5. One LGTM is enough: feel free to approve any outstanding CR.
  6. 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:

See also