Hi, 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: A 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? Thanks, Colin 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 parent_diff_file))) 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 and @@ -77,6 +78,7 @@ for f in self._process_files(parent_diff_file, basedir, check_existance=True): parent_files[f.origFile] = f + parent_rev = f.origInfo diffset = DiffSet(name=diff_file.name, revision=0, history=diffset_history, @@ -91,7 +93,10 @@ source_rev = parent_file.origInfo else: 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 reviewboard+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en -~----------~----~----~----~------~----~------~--~---
