Diff between Code review versions

Bottom line up front: create a tag with each version of a code review you post, to be able to see changes between versions.

Git commits come in (at least) three flavors.

First is the personal flavor, where you commit to git in order to not lose some quantum of behavior you have just implemented. These commits are small, and may be breaking commits. They are not meant for upstream consumption in the long term.


Second is the upstream commit, a reasonable chunk of a code to review. While the overall patch-series (to use the LKML term) might still be multiple patches, each one is meant to be stand-alone reviewed, and probably should be a non-breaking patch that can be merged as is…provided all the pre-req patches get merges.

The third type of commit is a fix to a code review commit. Say a patch series is 5 patches long, and you are asked to make a change in the first patch in the series. When doing your work, the first thing you do is make that change and make sure it works, and commit it on the top of the stack. Typically that change means you do a git rebase -i HEAD~6 (or comparable) and reorder the patches so that the fix sits just above the original commit you want to fix. At this point, you can squash the original and fixing commits together.

When using gitlab or github, you can update an existing code review request (Merge Request or MR in gitlab, Pull Request or PR in Github) by doing some form of git push with the –force flag. Since both git services use a branch to represent the request, an update to the branch will create an update to the request.

The problem with this process is that the change gets lost. Say that the change was questionable, and you just wanted to try it out. A fellow developer on a different system has no way of reverting to the previous stage…they would need the private information on your system.

Another code review system that does not lose this information is Gerrit. I don’t wish to compare and contrast all of the various merits and drawbacks of Gerrit. Instead, I wish to focus in on the fact that a review is a top level concept in Gerrit, and each version of a review becomes its own branch. A version of a review is visible from the server and you can diff between two versions of a review. I would like to have the option to do something similar, but in gitlab or github.

It turns out that you can, but it requires more work on the part of the developer. After you push each version of a merge request, you tag it and push that tag. They you update the merge request conversation with links to that tag.

Obviously, this requires more effort on the part of the coder, but since you are essentially performing two operations each time you push, you can script it. We can write a script git-pushrev that does the following:

  1. pushes the current head to gitlab with –force using the name of the current branch. Make sure you don;t use main or other protected branch.
  2. tags the current head. The name of the tag should be related to the name of the branch, with a 3 digit index appended
  3. push the current tag to git using git push tag <tagname>

The developer would need to then grab the tag and add it to the code review comments. However, if they forget to do this, the tag already exists and it could be looked up and posted in the future, or merely kept as a reference for later.

To see what work was done in a revision of a review, use git diff. For example, assuming a branch named code-review,

git diff code-review-000 code-review-001.

I have a project where I deleted a short file. Here is what I get:

ayoung@sut05sys-r112:~/testing/packaging$ git tag
plumbum-wip-0
plumbum-wip-1
ayoung@sut05sys-r112:~/testing/packaging$ git diff plumbum-wip-0 plumbum-wip-1
diff --git a/packaging/archive.py b/packaging/archive.py
deleted file mode 100644
index a3b2e94..0000000
--- a/packaging/archive.py
+++ /dev/null
@@ -1,6 +0,0 @@
-# Represents an archive to be built:  tar.gz, rpm, or comparable
-
-class Archive():
-    def __init__(self, kit_name, build_id, branch_tag, base_tag):
-        for key, value in locals().items():
-            setattr(self, key, value)

Leave a Reply

Your email address will not be published. Required fields are marked *

This site uses Akismet to reduce spam. Learn how your comment data is processed.