wiki:code-review

Version 1 (modified by nelhage@mit.edu, 10 years ago) (diff)

--

BarnOwl has a pre-commit code review policy. Essentially all code that goes to the master branch of the primary repository should be reviewed and ACKed by at least one other developer.
  • There is no official standard for who is allowed to ACK what. It suffices that both the author and the reviewer feel confident in their ability to evaluate the change.

    Larger changes, however, such as significant refactorings or design of non-trivial features, should be informed by broader discussion whenever possible.

  • An important part of code review is explicit communication from the reviewer. Review comments should include either a clear ACK, or one or more clear action items for the developer (generally of the form "make this change" or "answer this question").

  • The use of a "Reviewed-by:" line by the reviewer is suggested but not mandatory as an indicator that the reviewer is happy with the patch in its present form. Developers are encouraged but not required to include any applicable "Reviewed-by:" lines in the commit message of the patch when it is merged.

  • While code review of small patches may happen over Zephyr, or larger patches if a convenient reviewer is present and paying attention, decisions or review of non-trivial branches should, in general, happen at least largely over email (to barnowl-dev@mit.edu).

  • Review requests should include at least one of:

    • The location of a git repository containing the changes.
    • The patch(es) themselves inline in email (using e.g. git-send-email)
    In general, the former is preferred for long or complex patch series, and the latter for short patches.

  • If a review request to the mailing list is not responded to in any way within seven days from time of sending, the developer may, at their option, merge it to master without review. There is no such timeout for review requests communicated in any other way.