| | 1 | {{{ |
| | 2 | #!html |
| | 3 | BarnOwl has a pre-commit code review policy. Essentially all code that |
| | 4 | goes to the master branch of the primary repository should be reviewed |
| | 5 | and 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 | }}} |
