Changes between Initial Version and Version 1 of code-review


Ignore:
Timestamp:
Dec 25, 2009, 9:28:26 PM (12 years ago)
Author:
nelhage@mit.edu
Comment:

--

Legend:

Unmodified
Added
Removed
Modified
  • code-review

    v1 v1  
     1{{{
     2#!html
     3BarnOwl has a pre-commit code review policy. Essentially all code that
     4goes to the master branch of the primary repository should be reviewed
     5and ACKed by at least one other developer.
     6
     7<ul>
     8<li>
     9   <p>There is no official standard for who is allowed to ACK what. It
     10   suffices that both the author and the reviewer feel confident in their
     11   ability to evaluate the change.</p>
     12
     13   <p>Larger changes, however, such as significant refactorings or design of
     14   non-trivial features, should be informed by
     15   broader discussion whenever possible.</p>
     16</li>
     17<li><p>
     18   An important part of code review is explicit communication from the
     19   reviewer. Review comments should include either a clear ACK, or one
     20   or more clear action items for the developer (generally of the form
     21   "make this change" or "answer this question").
     22</p></li>
     23<li><p>
     24 The use of a "Reviewed-by:" line by the reviewer is suggested but
     25   not mandatory as an indicator that the reviewer is happy with the
     26   patch in its present form. Developers are encouraged but not
     27   required to include any applicable "Reviewed-by:" lines in the
     28   commit message of the patch when it is merged.
     29</p></li>
     30<li><p>
     31 While code review of small patches may happen over Zephyr, or
     32   larger patches if a convenient reviewer is present and paying
     33   attention, decisions or review of non-trivial branches should, in
     34   general, happen at least largely over email (to
     35   barnowl-dev@mit.edu).
     36</p></li>
     37<li><p>
     38 Review requests should include at least one of:
     39 <ul>
     40   <li>The location of a git repository containing the changes.</li>
     41   <li>The patch(es) themselves inline in email (using e.g. <tt>git-send-email</tt>)</li>
     42   </ul>
     43   In general, the former is preferred for long or complex patch series, and the latter for short patches.
     44</p></li>
     45<li><p>
     46 If a review request to the mailing list is not responded to in any
     47   way within seven days from time of sending, the developer may, at
     48   their option, merge it to master without review. There is no such
     49   timeout for review requests communicated in any other way.
     50</p></li>
     51</ul>
     52}}}