You cannot select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
100 lines
4.4 KiB
Plaintext
100 lines
4.4 KiB
Plaintext
The git-cl README describes the git-cl command set. This document
|
|
describes how code review and git work together in general, intended
|
|
for people familiar with git but unfamiliar with the code review
|
|
process supported by Rietveld.
|
|
|
|
== Concepts and terms
|
|
A Rietveld review is for discussion of a single change or patch. You
|
|
upload a proposed change, the reviewer comments on your change, and
|
|
then you can upload a revised version of your change. Rietveld stores
|
|
the history of uploaded patches as well as the comments, and can
|
|
compute diffs in between these patches. The history of a patch is
|
|
very much like a small branch in git, but since Rietveld is
|
|
VCS-agnostic the concepts don't map perfectly. The identifier for a
|
|
single review+patches+comments in Rietveld is called an "issue".
|
|
|
|
Rietveld provides a basic uploader that understands git. This program
|
|
is used by git-cl, and is included in the git-cl repo as upload.py.
|
|
|
|
== Basic interaction with git
|
|
The fundamental problem you encounter when you try to mix git and code
|
|
review is that with git it's nice to commit code locally, while during
|
|
a code review you're often requested to change something about your
|
|
code. There are a few different ways you can handle this workflow
|
|
with git:
|
|
|
|
1) Rewriting a single commit. Say the origin commit is O, and you
|
|
commit your initial work in a commit A, making your history like
|
|
O--A. After review comments, you commit --amend, effectively
|
|
erasing A and making a new commit A', so history is now O--A'.
|
|
(Equivalently, you can use git reset --soft or git rebase -i.)
|
|
|
|
2) Writing follow-up commits. Initial work is again in A, and after
|
|
review comments, you write a new commit B so your history looks
|
|
like O--A--B. When you upload the revised patch, you upload the
|
|
diff of O..B, not A..B; you always upload the full diff of what
|
|
you're proposing to change.
|
|
|
|
The Rietveld patch uploader just takes arguments to "git diff", so
|
|
either of the above workflows work fine. If all you want to do is
|
|
upload a patch, you can use the upload.py provided by Rietveld with
|
|
arguments like this:
|
|
|
|
upload.py --server server.com <args to "git diff">
|
|
|
|
The first time you upload, it creates a new issue; for follow-ups on
|
|
the same issue, you need to provide the issue number:
|
|
|
|
upload.py --server server.com --issue 1234 <args to "git diff">
|
|
|
|
== git-cl to the rescue
|
|
git-cl simplifies the above in the following ways:
|
|
|
|
1) "git cl config" puts a persistent --server setting in your .git/config.
|
|
|
|
2) The first time you upload an issue, the issue number is associated with
|
|
the current *branch*. If you upload again, it will upload on the same
|
|
issue. (Note that this association is tied to a branch, not a commit,
|
|
which means you need a separate branch per review.)
|
|
|
|
3) If your branch is "tracking" (in the "git checkout --track" sense)
|
|
another one (like origin/master), calls to "git cl upload" will
|
|
diff against that branch by default. (You can still pass arguments
|
|
to "git diff" on the command line, if necessary.)
|
|
|
|
In the common case, this means that calling simply "git cl upload"
|
|
will always upload the correct diff to the correct place.
|
|
|
|
== Patch series
|
|
The above is all you need to know for working on a single patch.
|
|
|
|
Things get much more complicated when you have a series of commits
|
|
that you want to get reviewed. Say your history looks like
|
|
O--A--B--C. If you want to upload that as a single review, everything
|
|
works just as above.
|
|
|
|
But what if you upload each of A, B, and C as separate reviews?
|
|
What if you then need to change A?
|
|
|
|
1) One option is rewriting history: write a new commit A', then use
|
|
git rebase -i to insert that diff in as O--A--A'--B--C as well as
|
|
squash it. This is sometimes not possible if B and C have touched
|
|
some lines affected by A'.
|
|
|
|
2) Another option, and the one espoused by software like topgit, is for
|
|
you to have separate branches for A, B, and C, and after writing A'
|
|
you merge it into each of those branches. (topgit automates this
|
|
merging process.) This is also what is recommended by git-cl, which
|
|
likes having different branch identifiers to hang the issue number
|
|
off of. Your history ends up looking like:
|
|
|
|
O---A---B---C
|
|
\ \ \
|
|
A'--B'--C'
|
|
|
|
Which is ugly, but it accurately tracks the real history of your work, can
|
|
be thrown away at the end by committing A+A' as a single "squash" commit.
|
|
|
|
In practice, this comes up pretty rarely. Suggestions for better workflows
|
|
are welcome.
|