Notes on building a distributed command-line code review system for native git.
Switch branches/tags
Nothing to show
Clone or download
Fetching latest commit…
Cannot retrieve the latest commit at this time.
Permalink
Type Name Latest commit message Commit time
Failed to load latest commit information.
conversation.txt
design.txt
readme.md

readme.md

git-revue

git-revue is a distributed code review tool for git.

It provides command-line and web interfaces. The web interface serves as both code browser and code review tool.

The name is a reference to a revue, which is in some sense a review of the thing set before the viewer. It was chosen largely because 'git-review' was already taken.

It also does not yet exist. These are just my design notes on how it might be built.

Design

Commits can be approved for merging into a branch. Approvals are signed, annotated tags. Their tag message contains the branch the commit is approved to merge into.

A given repository may be fitted with a pre-receive hook that rejects pushes that merge an unapproved commit ID into a protected branch. Branches are protected by configuring the pre-receive hook's list of protected branches. The hook can be set to allow merging commits that yield an identical patch to to merging an approved commit ID.

Anyone can use the web or command-line interfaces to leave a comment on a commit. A comment may apply to the whole commit, to a specific file in the commit, to specific lines in a commit (usually in one file), or even specific characters within a line in a commit. It might be useful to allow comments on diffs as such, too, and to treat the 'comment on commit' as a diff from commitref^..commitref?

A comment may stand alone or be in response to another comment.

diffscuss might be a good format for reading and writing reviews. They could be stored, pushed, and pulled as git notes or similar.

Implementation

The approval is signed with a single-purpose key created as part of admin setup. The key should be used solely for git-revue. It must have a password associated with it, so that even if someone malicious obtains the key, they cannot forge approvals.

Consider using custom git objects? With a custom protocol, pushing them around might be pretty simple, and we could perhaps avoid polluting the tag and branch lists. However, that leaves us at risk of breaking the rebase-following behavior.

Git notes could be used as a mechanism for storing review comments. A major drawback to notes is that you can only have one per commit.

Handling Rebases

When you rebase a branch, a git gc can delete the old commits.

If those commits have comments or approvals, letting that happen removes important historical data.

How can we prevent that from happening?

Tags are the obvious answer, but that would clutter things badly. If they only lived in the review repository, that might not be a disaster.

On pull and fetches, by default we should not return tags that were created solely for code review history preservation (unless you asked for them).

That could be achieved by storing review-related tags in namespaced refspecs, and having git-revue configure remotes with custom fetch configurations, so you don't retrieve a mountain of rebase-preservation tags unless you want them. We could also use custom commands as wrappers around push/fetch with custom refspecs, to keep the UI cleaner.

On pushes, we detect rebases and create history preservation tags as needed (along with any necessary metadata), in a namespace such as ref/tags/review-history.

That could be done with a pre-receive or update hook in the review repo - if a new ref is not a fast-forward, create the necessary history preservation tags. If anything fails, exit non-zero, so that history cannot be lost by accident.

This is not truly distributed - for that, the hooks would probably need to be pre-rebase (and other history-changing comands), but you would then have to push the tags manually, and you'd also wind up with a whole lot of tags.

What about cases where you need to nuke an old branch post-rebase (due to a copyright violation, for instance)? Well, should be simple enough - manually delete the references and demand git take out the garbage, per usual.

Existing Projects

git-appraise appears to cover a lot of this ground. I did most of the above design work before I realized it existed.

git-series looks like it does the heavy lifting around tracking rebases I want. It would be instructive to understand what approach it takes.

Feature Explorer is a proprietary tool used at a quant shop called Jane Street. It's pretty slick in a number of ways, judging from the blog post, and is reminiscent of the kind of review system I've long desired (though it does miss in a few places, I think, but it looks much better than most systems I've used).