My company is in the process of switching to Mercurial, and are using
ReviewBoard to do code reviews. I am trying to get parent diffs
working so that we can post reviews for changesets even if the source
revision is not in the master repository.

So in other words my master repository contains:


my local repo contains:

A -> B -> C

and I want someone to review the changes from B to C. So I post B-C as
the diff and A-B as the parent diff.

Looking at the post-review script it seems that this is currently
supported for git but not for Mercurial. With a bit of hacking in the
ReviewBoard code I've got it to work with Mercurial, but I want to
make sure that my solution isn't going to break anything else, as I
don't think I fully understand the model that ReviewBoard uses to
abstract the various supported SCM tools.

The first change I had to make was a simple bug fix (I think) that you
can see here: http://reviews.review-board.org/r/837/.

Even after this though, it doesn't work if the new diff includes files
that are not present in the parent diff (because they haven't changed
between A and B). In this case it uses B as the source revision for
these files, but because ReviewBoard doesn't know about B it fails
when trying to get the contents of the files.

So what I've done is modify the code in diffviewer/forms.py (see patch
below) so that if there is a parent diff, it uses the source revision
for the parent diff for all files (i.e. A) instead of the source
revision for the new diff.

This works great for Mercurial (the only issue is that the source
revision IDs displayed in the diff GUI are wrong, but that's not a
biggie), but I've no idea how this will affect the git implementation.
Does anyone have any suggestions?


Index: forms.py
--- forms.py    (revision 1931)
+++ forms.py    (working copy)
@@ -60,7 +60,7 @@

         # Parse the diff
         files = list(self._process_files(
-            diff_file, basedir, check_existance=(parent_diff_file is
not None)))
+            diff_file, basedir, check_existance=(not

         if len(files) == 0:
             raise EmptyDiffError(_("The diff file is empty"))
@@ -70,6 +70,7 @@

         # Parse the parent diff
         parent_files = {}
+        parent_rev = None

         if parent_diff_file:
             # If the user supplied a base diff, we need to parse it
@@ -77,6 +78,7 @@
             for f in self._process_files(parent_diff_file, basedir,
                 parent_files[f.origFile] = f
+                parent_rev = f.origInfo

         diffset = DiffSet(name=diff_file.name, revision=0,
@@ -91,7 +93,10 @@
                 source_rev = parent_file.origInfo
                 parent_content = ""
-                source_rev = f.origInfo
+                if parent_rev and f.origInfo != PRE_CREATION:
+                    source_rev = parent_rev
+                else:
+                    source_rev = f.origInfo

             dest_file = os.path.join(basedir, f.newFile).replace("\
\", "/")

You received this message because you are subscribed to the Google Groups 
"reviewboard" group.
To post to this group, send email to reviewboard@googlegroups.com
To unsubscribe from this group, send email to 
For more options, visit this group at 

Reply via email to