RFC: supporting code review and partial commits better

[Scott Scriven]

* Robert Collins <robertc at robertcollins.net> wrote:
> One of the reasons I prefer to review in my email client is 
> that I can achieve this marking of bits as 'good' by deleting 
> them in my reply.

Email is rather nice for this.  It turns the diff into a 
discussion, with threads for individual bits of code.

Have you ever seen Review Board?  It has some good UI ideas for 
handling reviews via the web, and I'd love to see some of its 
features in launchpad...

  http://www.review-board.org/

The main thing is it provides some of the same benefits as 
email-based reviews, like discussions per line of diffs and 
multiple revisions of each patch.

> So - I would love the ability to mark (in varying granularity 
> down to the hunk level) parts of the workingtree<->basis 
> difference with a tag of some sort.

Sometimes hunk-level changes are too large.

For example, say someone adds two menu items to a program, one 
good and one bad.  The reviewer wants to mark the good one as 
approved and the bad one as denied.  Here's a sample patch which 
doesn't work with a hunk-based approach:

--- file.orig  2008-08-28 07:23:49.118539866 -0600
+++ file.new   2008-08-28 07:24:43.669648559 -0600
@@ -2,6 +2,8 @@
 
 menu = [
     ("item1", item1_handler),
+    ("new_good_item", new_good_item_handler),
+    ("new_bad_item", new_bad_item_handler),
     ("item2", item2_handler),
     ]
 
@@ -10,3 +12,9 @@
 
 def item2_handler():
     pass
+
+def new_good_item_handler():
+    pass
+
+def new_bad_item_handler():
+    pass


I think the ideas you described are good, but I'd suggest adding 
the ability to split hunks somehow.


-- Scott