InDefero

Sign in or create your account | Project List | Help

Issue 39: Improving the code review

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.
Status: Accepted

Comment 3 by Loïc d'Anterroches, Nov 24, 2008

Ok, found a way to do it.

The principle will be to have a patch against a given revision of the
code, the patch can be in the form of an already applied commit (we
have the diff from the commit) view or from an uploaded patch.

Post commit: you select a commit/revision in the source view and you
click on a "Start review" button. We have all the base data for the
review.

Pre commit: you go in the code review tab, give the reference
commit/revision, upload a patch.

For both pre/post commit, you give a little description and provide a
list of people you want to review your code.

You submit the form. A review request is created and emails to the
invited reviewers are sent.

On the review page, you can see on the left the set of original files
and on the right the new version, everything with red/green colours
to highlight the diffs.

The reviewers and the submitters can easily had comments, either at a
line level or at the complete file level. The comments are "pending"
until the end of the review. When the reviewer has finished, he can
"publish his review" and the comments are made available to others,
an email is also sent to the submitter of the review.

From a technical point of view, the only difficult part is to apply
the patch in the case of a pre commit review. We want to be able to
apply a path without the need to run the patch program.

From a GUI point of view, it will requires javascript for the inline
comments. Full file comments will not require javascript. I will make
it so Javascript is *not* mandatory, that way people without
javascript will still be able to put full file comments.
Labels: Priority:Medium, -Priority:Low

Comment 4 by Loïc d'Anterroches, Nov 30, 2008

Started in commit f690968b1149970a989bfa. Now that the basis is
there, it requires polishing.
Status: Started
Owner: loic

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.
Labels: Type:Defect, -Type:Enhancement
Summary: Improving the code review

Comment 6 by Loïc d'Anterroches, Dec 2, 2008

(No comments were given for this change.)
Labels: Type:Enhancement, Milestone:Release1.0, -Type:Defect

Created: 4 months 9 days ago by Loïc d'Anterroches

Updated: 1 month 5 days ago

Status: Started

Owner: Loïc d'Anterroches

Labels:
Type:Enhancement
Priority:Medium
Milestone:Release1.0