google / git-appraise

Distributed code review system for Git repos
Apache License 2.0
5.13k stars 146 forks source link

Support comments that are detached from any review. #97

Closed ojarjur closed 4 years ago

ojarjur commented 4 years ago

The git-appraise tool supports commenting on arbitrary locations in the repository, but it only does so in the context of reviewing a commit.

It would be nice if we could also support commenting on arbitrary locations in the source code without having to wait for a code review.

For example, this could allow someone to pair git-appraise with the git-sync-changes tool to build real-time pair-programming tools on top of git.

My initial thoughts on how this could be done:

  1. Extend git appraise comment by adding a -d flag to indicate that the comment is attached to any review.
  2. Extend the git appraise show command by allowing you to specify a file path instead of a review hash, and then having it show you the detached comments for that path.

Since git-notes always have to be attached to a git-object, we could define a fixed, well-known object to anchor these detached comments. For example, we could pick a "zero" commit of 71ee2d2204443f001fde5aa510ec956d7f90e5e1, which can always be recreated with the following command:

echo "" | \
    GIT_AUTHOR_NAME="nobody" \
    GIT_AUTHOR_EMAIL="nobody" \
    GIT_AUTHOR_DATE="100000000 +0000" \
    GIT_COMMITTER_NAME="nobody" \
    GIT_COMMITTER_EMAIL="nobody" \
    GIT_COMMITTER_DATE="100000000 +0000" \
    git commit-tree `git hash-object -t tree /dev/null`

We could then save that commit to a ref like refs/devtools/archives/71ee2d2204443f001fde5aa510ec956d7f90e5e1 to make sure it does not get garbage collected.

When adding a comment for a path, we would create a new comment struct for that path at the specified commit (defaulting to HEAD if no commit is specified), serialize it, and add it to the git notes for the well-known commit under refs/notes/discuss.

When listing the comments for a path, we would parse the git-notes for the well-known commit under refs/notes/discuss, and then filter out any whose location does not match the provided path.

That would not preserve comments after a file rename, but a caller could account for this by computing the previous file names using the command git log --follow --name-only --pretty=format:"" ${FILENAME}, and then listing all of the detached comments for every previous path.

There is a risk that the use of a single well-known commit might pose a scaling bottleneck, so we will have to load test this with lots of comments to see how well it scales. If that is a issue, then we could use a different, reproducible commit object for each path (such as by adding a message to the otherwise empty commit specifying the matching path).