Reported by Loïc d'Anterroches, Aug 29, 2008
A nice feature would be code review. GoogleCode has introduced that, Rietveld is also available: http://codereview.appspot.com/ I have tried the GoogleCode version and have a bad "feeling" about it. The interface is confusing with many ways to give feedback and the multiple steps to add comments (draft then publish). We can do better :)
Comment 1 by Xavier Brochard, Sep 11, 2008
Hello Loïc, I was browsing here to add the same wish. I've just discovered Review Board http://www.review-board.org/ It's in PHP and use the Django framework.
Comment 2 by Loïc d'Anterroches, Sep 12, 2008
Hi Xavier, yes, this review board is quite nice, my question at the moment is more a problem of the way to handle the review requests. A review is basically a request to check that a diff/patch is good enough to be included within the mainline and we need to have the tools to easily comment and improve until we reach something right. My idea at the moment is to request the submitter of a review to upload a diff/patch against a given git commit or subversion revision. Then it will be possible to attach to the diff/patch some general comments, like here about the issues or line by line comments (using a kind of AJAX interface). Still not fully sure about the approach, but I want it to be very simple for both the people submitting the review request and the reviewer.
Comment 3 by Loïc d'Anterroches, Nov 24, 2008
Comment 4 by Loïc d'Anterroches, Nov 30, 2008
Started in commit f690968b1149970a989bfa. Now that the basis is there, it requires polishing.
Comment 5 by Loïc d'Anterroches, Nov 30, 2008
What needs to be done for the code review: - Notifications for the reviews. - Select a commit to create a code review. - Update a code review with a new patch. - Change the status of a code review.
Summary: Improving the code review
Comment 6 by Loïc d'Anterroches, Dec 2, 2008
Comment 7 by Andrey Kulikov, May 28, 2009
It'll be VERY useful to make possible to attach comments to each separate line, not only just to the whole changed file. This feature exist in CodeCollaborator (comercial tool). I've use it, and found it very convenient.
Comment 8 by Loïc d'Anterroches, Jun 29, 2009
<chuck> da-loic: I created some simple-ish CSS/JS magic for doing per-line comments in code review if you want <da-loic> chuck: yes yes yes <chuck> da-loic: kk, it's not completely finished, but here's a demo (http://melbye.nfshost.com/codereview/) and here's the archive of it (http://melbye.nfshost.com/codereview.tar.gz) <da-loic> chuck: great, adding that to the ticket
Comment 9 by Charles Melbye, Jun 29, 2009
The archive of a library to manage comments on a per line basis is attached. It's in its preliminary stages, and it might have to be refactored a bit. For example, it doesn't actually know how to display the actual comments yet, but that should be the easy part.
- codereview.tar.gz - 2.26 kB
Comment 10 by Loïc d'Anterroches, Jul 15, 2009
Working an it now, my steps are: 1. get the same functionalities as now but fixing issue 145 and adding the ability to propose new patches for a given review. 2. get the comments at the line level, the backend will have the support of the lines already at step 1, this means it will be possible to use the reviews safely when step 1 will be done.
Comment 11 by Loïc d'Anterroches, Jul 15, 2009
I have the new DB schema definition, I now need to update the UI to match it. I will make a code sprint to get that done tonight. Considered as a high priority task.
Comment 12 by Julien Huang, Oct 30, 2009
My 2 cents, there may be some ideas/concepts to get from Gerrit. Rietveld from Google has been forked (by Google also) into "Gerrit" (http://gerrit.googlecode.com/). Gerrit is being used for the Android OS project : https://review.source.android.com/ The submission workflow ("Gerrit inside") : http://source.android.com/submit-patches http://source.android.com/submit-patches/workflow
Comment 13 by Loïc d'Anterroches, Oct 30, 2009
Great, especially the last two links. Thanks!
Comment 15 by Loïc d'Anterroches, Feb 14, 2010