|
| 1 | +The git-cl README describes the git-cl command set. This document |
| 2 | +describes how code review and git work together in general, intended |
| 3 | +for people familiar with git but unfamiliar with the code review |
| 4 | +process supported by Rietveld. |
| 5 | + |
| 6 | +== Concepts and terms |
| 7 | +A Rietveld review is for discussion of a single change or patch. You |
| 8 | +upload a proposed change, the reviewer comments on your change, and |
| 9 | +then you can upload a revised version of your change. Rietveld stores |
| 10 | +the history of uploaded patches as well as the comments, and can |
| 11 | +compute diffs in between these patches. The history of a patch is |
| 12 | +very much like a small branch in git, but since Rietveld is |
| 13 | +VCS-agnostic the concepts don't map perfectly. The identifier for a |
| 14 | +single review+patches+comments in Rietveld is called an "issue". |
| 15 | + |
| 16 | +Rietveld provides a basic uploader that understands git. This program |
| 17 | +is used by git-cl, and is included in the git-cl repo as upload.py. |
| 18 | + |
| 19 | +== Basic interaction with git |
| 20 | +The fundamental problem you encounter when you try to mix git and code |
| 21 | +review is that with git it's nice to commit code locally, while during |
| 22 | +a code review you're often requested to change something about your |
| 23 | +code. There are a few different ways you can handle this workflow |
| 24 | +with git: |
| 25 | + |
| 26 | +1) Rewriting a single commit. Say the origin commit is O, and you |
| 27 | + commit your initial work in a commit A, making your history like |
| 28 | + O--A. After review comments, you commit --amend, effectively |
| 29 | + erasing A and making a new commit A', so history is now O--A'. |
| 30 | + (Equivalently, you can use git reset --soft or git rebase -i.) |
| 31 | + |
| 32 | +2) Writing follow-up commits. Initial work is again in A, and after |
| 33 | + review comments, you write a new commit B so your history looks |
| 34 | + like O--A--B. When you upload the revised patch, you upload the |
| 35 | + diff of O..B, not A..B; you always upload the full diff of what |
| 36 | + you're proposing to change. |
| 37 | + |
| 38 | +The Rietveld patch uploader just takes arguments to "git diff", so |
| 39 | +either of the above workflows work fine. If all you want to do is |
| 40 | +upload a patch, you can use the upload.py provided by Rietveld with |
| 41 | +arguments like this: |
| 42 | + |
| 43 | + upload.py --server server.com <args to "git diff"> |
| 44 | + |
| 45 | +The first time you upload, it creates a new issue; for follow-ups on |
| 46 | +the same issue, you need to provide the issue number: |
| 47 | + |
| 48 | + upload.py --server server.com --issue 1234 <args to "git diff"> |
| 49 | + |
| 50 | +== git-cl to the rescue |
| 51 | +git-cl simplifies the above in the following ways: |
| 52 | + |
| 53 | +1) "git cl config" puts a persistent --server setting in your .git/config. |
| 54 | + |
| 55 | +2) The first time you upload an issue, the issue number is associated with |
| 56 | + the current *branch*. If you upload again, it will upload on the same |
| 57 | + issue. (Note that this association is tied to a branch, not a commit, |
| 58 | + which means you need a separate branch per review.) |
| 59 | + |
| 60 | +3) If your branch is "tracking" (in the "git checkout --track" sense) |
| 61 | + another one (like origin/master), calls to "git cl upload" will |
| 62 | + diff against that branch by default. (You can still pass arguments |
| 63 | + to "git diff" on the command line, if necessary.) |
| 64 | + |
| 65 | +In the common case, this means that calling simply "git cl upload" |
| 66 | +will always upload the correct diff to the correct place. |
| 67 | + |
| 68 | +== Patch series |
| 69 | +The above is all you need to know for working on a single patch. |
| 70 | + |
| 71 | +Things get much more complicated when you have a series of commits |
| 72 | +that you want to get reviewed. Say your history looks like |
| 73 | +O--A--B--C. If you want to upload that as a single review, everything |
| 74 | +works just as above. |
| 75 | + |
| 76 | +But what if you upload each of A, B, and C as separate reviews? |
| 77 | +What if you then need to change A? |
| 78 | + |
| 79 | +1) One option is rewriting history: write a new commit A', then use |
| 80 | + git rebase -i to insert that diff in as O--A--A'--B--C as well as |
| 81 | + squash it. This is sometimes not possible if B and C have touched |
| 82 | + some lines affected by A'. |
| 83 | + |
| 84 | +2) Another option, and the one espoused by software like topgit, is for |
| 85 | + you to have separate branches for A, B, and C, and after writing A' |
| 86 | + you merge it into each of those branches. (topgit automates this |
| 87 | + merging process.) This is also what is recommended by git-cl, which |
| 88 | + likes having different branch identifiers to hang the issue number |
| 89 | + off of. Your history ends up looking like: |
| 90 | + |
| 91 | + O---A---B---C |
| 92 | + \ \ \ |
| 93 | + A'--B'--C' |
| 94 | + |
| 95 | + Which is ugly, but it accurately tracks the real history of your work, can |
| 96 | + be thrown away at the end by committing A+A' as a single "squash" commit. |
| 97 | + |
| 98 | +In practice, this comes up pretty rarely. Suggestions for better workflows |
| 99 | +are welcome. |
0 commit comments