Updated Issue 39 - Improving the code review (indefero)

[no-r...@ceondo.com]

Hello,

The following issue has been updated:

39 - Improving the code review
Project: Indefero
Status: Started
Reported by: Loïc d'Anterroches
URL: http://projects.ceondo.com/p/indefero/issues/39/
Labels:
Priority:High
Type:Enhancement

Comments (last first):

# By Mathias Bavay, Sep 4, 2013:

Creating patches for code reviews is not always obvious: some users do a diff between two checkouts and this does not work. A line at the bottom of screen (like in for "source" tab for doing checkouts) could specify something like "svn diff > my.patch" as an example of how to create a patch suitable for code review.

# By Loïc d'Anterroches, Feb 14, 2010:



Labels: -Milestone:Release1.0

# By Jakub Vitak, Nov 2, 2009:

please see Issue 326

# By Loïc d'Anterroches, Oct 30, 2009:

Great, especially the last two links. Thanks!

# 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

# 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.

Labels: Priority:High -Priority:Medium

# 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.

# 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.

Attachments:
- codereview.tar.gz - 2.26 kB
http://projects.ceondo.com/p/indefero/issues/view/attachment/64/codereview.tar.gz


# 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

# 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.

# By Loïc d'Anterroches, Dec 2, 2008:



Labels: Type:Enhancement Milestone:Release1.0 -Type:Defect

# 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

# 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

# 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

# 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

# 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.

# 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 :)



--
Issue: http://projects.ceondo.com/p/indefero/issues/39/