Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(185)

Side by Side Diff: README.codereview

Issue 2232303002: Overhaul the README files a bit. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/tools/depot_tools
Patch Set: Use better example Created 4 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
« no previous file with comments | « no previous file | README.gclient » ('j') | README.gclient.md » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
(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.
OLDNEW
« no previous file with comments | « no previous file | README.gclient » ('j') | README.gclient.md » ('J')

Powered by Google App Engine
This is Rietveld 408576698