DaalaReview

From XiphWiki
Revision as of 11:21, 19 June 2013 by Derf (talk | contribs)
Jump to navigation Jump to search
The printable version is no longer supported and may have rendering errors. Please update your browser bookmarks and please use the default browser print function instead.

This is a proposal for a more flexible review process. It is a set of guidelines for the most appropriate approach for different types of changes. Above all, use your judgement.

Definitions

  • A) Full review: Same as current process. Can be based on Rietveld, "git format-patch", or a pull request
  • B) Post-review: Code gets committed to repo first, committer nags reviewer weekly if necessary
  • C) Design review: Discuss with original author (and possibly others) for the principle of a change, can (and probably should) be done even before doing the work
  • D) No review: self-explanatory

Guidelines

  • 0) Full-review on anything the committer has reasons to believe would be controversial, buggy, ...
  • 1) Generally no review on comments/whitespace/style/documentation
    • Except potentially when changing Doxygen API comments
  • 2)Post-review on code that is already badly broken, or on simple bug fixes
  • 3) No review on initial commit of new stuff that isn't being called
    • Full-review required to hook any code not reviewed because of 3)
    • If code in 3) breaks the build, it gets fixed immediately or removed
    • No review on new tools until they get used by other people
  • 4) Code that is "obviously correct" or that is trivially testable gets post-review
  • 5) Code that may have subtle implications gets full review
  • 6) Refactoring of someone else's code gets design review, followed by either post-review (or full review if requested during design review)
  • 7) Disruptive work (e.g. block switching) gets both design review and full review.
  • 8) Most build system changes get no review (unless 0) applies)
  • 9) Post-review on test code

Examples

1) da726b32, 53362b77 2) 212d13b3, 3) 76ee32c8, 75b62b23 4) 7c102188, 50732d66 5) 8cd46564, 74dc3616 6) 208ea300 7) b4bfee65 8) 7cc3cc5f, 487fe063

Team reviews

  • 1) Once in a while, we should pick a piece of code
  • 2) Original author cleans up code, adds comments
  • 3) Code discussed with entire team
  • 4) Suggestions for improvement (with emphasis on algo)