OLD | NEW |
| (Empty) |
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. | |
OLD | NEW |