At [https://trac.webkit.org/wiki/April%202010%20Meeting April 2010 Meeting] we talked about what is good and sad about the current state of code reviews. Here's some points that showed up: == Bugzilla Advantages == Things people don't want to lose. - Strong link between review and bugs and commit queue - integration - Comments are emailed out automatically - Full pretty printed diff - Patch history - Web based - Intra-line diff hightlight - Command line access to initiate review - Very low downtime - Low incompatiblilities - Fast - Multiple unrelated patches in a single bug == Bugzilla Caveats == If you want to fix any of these, file a bug and post patch there and update this wiki with a link to the bug so they can be removed. - Inefficient line by line review - Diff between "patches revision" inside a bug - General unrefinement of bugzilla - No draft saving - No statistics about the patch properties on the bug report page (number of lines, new layout tests, etc) - Navigate the patch at the file level - Lack of context for each diff chunk - More granularity in "review result", to improve the clarity of the communication, like "r+ but with small changes" - Unclear accronyms (what does r+ means?), ramping up. Use more English words (?) - Review progress, especially in the case of multiple reviewers (who review which parts) - Lack of key bindings - Review on a cell phone / tablet (at least basic tasks) - It would be nice if there was only one account - DO NOT add a third account - Email based review - Syntax based highlighting == Don't care == Didn't seem to have interest in the room. - Review on top of review, on top of patch - Full commandline access - Single patch affecting multiple bugs == Requirements for replacement == - Keep similar process, integration in bugzilla == Action Items == - Prototype, prefer a dual-review system during transition, if a transition ever occurs. - Any prototype should update back bugzilla to stay in sync. - Fixes to bugzilla work flow are welcome == Alternatives == - Just incrementally fix caveats in bugzilla. Here's an example to start off: http://blog.fishsoup.net/2009/09/23/splinter-patch-review. - [http://code.google.com/p/rietveld Rietveld]. Nice, as some issues and requires significant integration effort. - [http://code.google.com/p/gerrit Gerrit2]. Nice but requires [UsingGitWithWebKit git]. - [http://www.reviewboard.org Review board]. Some [https://bugzilla.mozilla.org/show_bug.cgi?id=515210 people pushing Mozilla to use it] but no progress.