| | 1 | 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: |
| | 2 | |
| | 3 | Bugzilla Advantages: |
| | 4 | - Strong link between review and bugs and commit queue - integration |
| | 5 | - Comments are emailed out automatically |
| | 6 | - Full pretty printed diff |
| | 7 | - Patch history |
| | 8 | - Web based |
| | 9 | - Intra-line diff hightlight |
| | 10 | - Command line access to initiate review |
| | 11 | - Very low downtime |
| | 12 | - Low incompatiblilities |
| | 13 | - Fast |
| | 14 | - Multiple unrelated patches in a single bug |
| | 15 | |
| | 16 | Bugzilla Caveats: |
| | 17 | - Inefficient line by line review |
| | 18 | - Diff between "patches revision" inside a bug |
| | 19 | - General unrefinement of bugzilla |
| | 20 | - No draft saving |
| | 21 | - No statistics about the patch properties on the bug report page (number of lines, new layout tests, etc) |
| | 22 | - Navigate the patch at the file level |
| | 23 | - Lack of context for each diff chunk |
| | 24 | - More granularity in "review result", to improve the clarity of the communication, like "r+ but with small changes" |
| | 25 | - Unclear accronyms (what does r+ means?), ramping up. Use more English words (?) |
| | 26 | - Review progress, especially in the case of multiple reviewers (who review which parts) |
| | 27 | - Lack of key bindings |
| | 28 | - Review on a cell phone / tablet (at least basic tasks) |
| | 29 | - It would be nice if there was only one account - DO NOT add a third account |
| | 30 | - Email based review |
| | 31 | - Syntax based highlighting |
| | 32 | |
| | 33 | Don't care: |
| | 34 | - Review on top of review, on top of patch |
| | 35 | - Full commandline access |
| | 36 | - Single patch affecting multiple bugs |
| | 37 | |
| | 38 | Requirements for replacement: |
| | 39 | - Keep similar process, integration in bugzilla |
| | 40 | |
| | 41 | Action Items: |
| | 42 | - Prototype, prefer a dual-review system during transition, if a transition ever occurs. |
| | 43 | - Any prototype should update back bugzilla to stay in sync. |
| | 44 | - Fixes to bugzilla work flow are welcome |
| | 45 | |
| | 46 | Alternatives: |
| | 47 | - Just incrementally fix caveats in bugzilla. Here's an example to start off: http://blog.fishsoup.net/2009/09/23/splinter-patch-review. |
| | 48 | - [http://code.google.com/p/rietveld Rietveld]. Nice, as some issues and requires significant integration effort. |
| | 49 | - [http://code.google.com/p/gerrit Gerrit2]. Nice but requires [UsingGitWithWebKit git]. |
| | 50 | - [http://www.reviewboard.org Review board]. Some people pushing Mozilla to use it. See https://bugzilla.mozilla.org/show_bug.cgi?id=515210. |
