DaalaReview

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 point 3
- If code in point 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 (according to the committer) highly unlikely to break things 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 point 0 applies)
9) Post-review on test code

Examples

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)

DaalaReview

Contents

Definitions

Guidelines

Examples

Team reviews

Navigation menu