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

Side by Side Diff: site/dev/contrib/submit.md

Issue 1411403010: Documentation - Remove references to `gclient sync` (Closed) Base URL: https://skia.googlesource.com/skia.git@master
Patch Set: 2015-11-09 (Monday) 11:38:05 EST Created 5 years, 1 month 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
« no previous file with comments | « no previous file | site/user/download.md » ('j') | site/user/download.md » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 How to submit a patch 1 How to submit a patch
2 ===================== 2 =====================
3 3
4 4
5 Configure git
6 -------------
7
8 <!--?prettify lang=sh?-->
9
10 $ git config --global user.name "Your Name"
11 $ git config --global user.email you@example.com
12
5 Making changes 13 Making changes
6 -------------- 14 --------------
7 15
8 First create a branch for your changes: 16 First create a branch for your changes:
9 17
10 ~~~~ 18 $ git config branch.autosetuprebase always
hcm 2015/11/09 18:23:55 As discussed, either note this as optional and/or
hal.canary 2015/11/09 19:45:50 done
11 $ git checkout -b my_feature origin/master 19 $ git checkout -b my_feature origin/master
12 ~~~~
13 20
14 After making your changes, create a commit 21 After making your changes, create a commit
15 22
16 ~~~~ 23 $ git add [file1] [file2] ...
17 $ git add [file1] [file2] ... 24 $ git commit
18 $ git commit
19 ~~~~
20 25
21 If your branch gets out of date, you will need to update it: 26 If your branch gets out of date, you will need to update it:
22 27
23 ~~~~ 28 $ git pull --rebase
24 $ git pull --rebase 29 $ python bin/sync-and-gyp
25 $ gclient sync
26 ~~~~
27 30
31 <!--
32 $ python tools/git-sync-deps
33 $ python ./gyp_skia
34 -->
28 35
29 Adding a unit test 36 Adding a unit test
30 ------------------ 37 ------------------
31 38
32 If you are willing to change Skia codebase, it's nice to add a test at the same 39 If you are willing to change Skia codebase, it's nice to add a test at the same
33 time. Skia has a simple unittest framework so you can add a case to it. 40 time. Skia has a simple unittest framework so you can add a case to it.
34 41
35 Test code is located under the 'tests' directory. 42 Test code is located under the 'tests' directory.
36 43
37 See [Writing Unit and Rendering Tests](tests) for details. 44 See [Writing Unit and Rendering Tests](../testing/tests) for details.
38 45
39 Unit tests are best, but if your change touches rendering and you can't think of 46 Unit tests are best, but if your change touches rendering and you can't think of
40 an automated way to verify the results, consider writing a GM test or a new page 47 an automated way to verify the results, consider writing a GM test or a new page
41 of SampleApp. Also, if your change is the GPU code, you may not be able to write 48 of SampleApp. Also, if your change is the GPU code, you may not be able to write
42 it as part of the standard unit test suite, but there are GPU-specific testing 49 it as part of the standard unit test suite, but there are GPU-specific testing
43 paths you can extend. 50 paths you can extend.
44 51
45 Submitting a patch 52 Submitting a patch
46 ------------------ 53 ------------------
47 54
48 For your code to be accepted into the codebase, you must complete the 55 For your code to be accepted into the codebase, you must complete the
49 [Individual Contributor License 56 [Individual Contributor License
50 Agreement](http://code.google.com/legal/individual-cla-v1.0.html). You can do 57 Agreement](http://code.google.com/legal/individual-cla-v1.0.html). You can do
51 this online, and it only takes a minute. If you are contributing on behalf of a 58 this online, and it only takes a minute. If you are contributing on behalf of a
52 corporation, you must fill out the [Corporate Contributor License Agreement](htt p://code.google.com/legal/corporate-cla-v1.0.html) 59 corporation, you must fill out the [Corporate Contributor License
53 and send it to us as described on that page. Add your (or your organization's) 60 Agreement](http://code.google.com/legal/corporate-cla-v1.0.html)
61 and send it to us as described on that page. Add your (or your organization's)
54 name and contact info to the AUTHORS file as a part of your CL. 62 name and contact info to the AUTHORS file as a part of your CL.
55 63
56 Now that you've made a change and written a test for it, it's ready for the code 64 Now that you've made a change and written a test for it, it's ready for the code
57 review! Submit a patch and getting it reviewed is fairly easy with depot tools. 65 review! Submit a patch and getting it reviewed is fairly easy with depot tools.
58 66
59 Use git-cl, which comes with [depot tools](http://sites.google.com/a/chromium.or g/dev/developers/how-tos/install-depot-tools). 67 Use git-cl, which comes with [depot
68 tools](http://sites.google.com/a/chromium.org/dev/developers/how-tos/install-dep ot-tools).
60 For help, run git-cl help. 69 For help, run git-cl help.
61 70
62 ### Configuring git-cl 71 ### Configuring git-cl
63 72
64 Before using any git-cl commands you will need to configure it to point at the 73 Before using any git-cl commands you will need to configure it to point at the
65 correct code review server. This is accomplished with the following command: 74 correct code review server. This is accomplished with the following command:
66 75
67 ~~~~ 76 git cl config https://skia.googlesource.com/skia/+/master/codereview.setting s
68 git cl config https://skia.googlesource.com/skia/+/master/codereview.settings
69 ~~~~
70 77
71 ### Find a reviewer 78 ### Find a reviewer
72 79
73 Ideally, the reviewer is someone who is familiar with the area of code you are 80 Ideally, the reviewer is someone who is familiar with the area of code you are
74 touching. If you have doubts, look at the git blame for the file to see who else 81 touching. If you have doubts, look at the git blame for the file to see who else
75 has been editing it. 82 has been editing it.
76 83
77 ### Uploading changes for review 84 ### Uploading changes for review
78 85
79 Skia uses Chromium's code review [site](http://codereview.chromium.org) and the 86 Skia uses Chromium's code review [site](http://codereview.chromium.org) and the
80 Rietveld open source code review tool. 87 Rietveld open source code review tool.
81 Use git cl to upload your change: 88 Use git cl to upload your change:
82 ~~~~ 89
83 $ git cl upload 90 $ git cl upload
84 ~~~~
85 91
86 You may have to enter a Google Account username and password to authenticate 92 You may have to enter a Google Account username and password to authenticate
87 yourself to codereview.chromium.org. A free gmail account will do fine, or any 93 yourself to codereview.chromium.org. A free gmail account will do fine, or any
88 other type of Google account. It does not have to match the email address you 94 other type of Google account. It does not have to match the email address you
89 configured using git config --global user.email above, but it can. 95 configured using `git config --global user.email` above, but it can.
90 96
91 The command output should include a URL, similar to 97 The command output should include a URL, similar to
92 (https://codereview.chromium.org/111893004/), indicating where your changelist 98 (https://codereview.chromium.org/111893004/), indicating where your changelist
93 can be reviewed. 99 can be reviewed.
94 100
95 ### Request review 101 ### Request review
96 102
97 Go to the supplied URL or go to the code review page and click **Issues created 103 Go to the supplied URL or go to the code review page and click **Issues created
98 by me**. Select the change you want to submit for review and click **Edit 104 by me**. Select the change you want to submit for review and click **Edit
99 Issue**. Enter at least one reviewer's email address and click **Update Issue**. 105 Issue**. Enter at least one reviewer's email address and click **Update Issue**.
(...skipping 23 matching lines...) Expand all
123 129
124 You will likely get email back from the reviewer with comments. Fix these and 130 You will likely get email back from the reviewer with comments. Fix these and
125 update the patch set in the issue by uploading again. The upload will explain 131 update the patch set in the issue by uploading again. The upload will explain
126 that it is updating the current CL and ask you for a message explaining the 132 that it is updating the current CL and ask you for a message explaining the
127 change. Be sure to respond to all comments before you request review of an 133 change. Be sure to respond to all comments before you request review of an
128 update. 134 update.
129 135
130 If you need to update code the code on an already uploaded CL, simply edit the 136 If you need to update code the code on an already uploaded CL, simply edit the
131 code, commit it again locally, and then run git cl upload again e.g. 137 code, commit it again locally, and then run git cl upload again e.g.
132 138
133 ~~~~ 139 $ echo "GOATS" > whitespace.txt
134 echo "" > GOATS 140 $ git add whitespace.txt
135 git add GOATS 141 $ git commit -m 'add GOATS fix to whitespace.txt'
136 git commit -m 'add newline fix to GOATS' 142 $ git cl upload
137 git cl upload
138 ~~~~
139 143
140 Once you're ready for another review, use **Publish+Mail Comments** again to 144 Once you're ready for another review, use **Publish+Mail Comments** again to
141 send another notification (it is helpful to tell the review what you did with 145 send another notification (it is helpful to tell the review what you did with
142 respect to each of their comments). When the reviewer is happy with your patch, 146 respect to each of their comments). When the reviewer is happy with your patch,
143 they will say "LGTM" ("Looks Good To Me"). 147 they will say "LGTM" ("Looks Good To Me").
144 148
145 _Note_: As you work through the review process, both you and your reviewers 149 _Note_: As you work through the review process, both you and your reviewers
146 should converse using the code review interface, and send notes using 150 should converse using the code review interface, and send notes using
147 **Publish+Mail Comments**. 151 **Publish+Mail Comments**.
148 152
153 Once your change has received an LGTM ("looks good to me"), you can check the
hcm 2015/11/09 18:23:55 okay to not spell out "looks good to me" again sin
hal.canary 2015/11/09 19:45:50 done
154 "Commit" box on the codereview page and it will be committed on your behalf.
155
149 Once your commit has gone in, you should delete the branch containing your chang e: 156 Once your commit has gone in, you should delete the branch containing your chang e:
150 157
151 ~~~~ 158 $ git checkout -q origin/master
152 $ git checkout master 159 $ git branch -D my_feature
153 $ git branch -D my_feature
154 ~~~~
155 160
156 161
157 Final Testing 162 Final Testing
158 ------------- 163 -------------
159 164
160 Skia's principal downstream user is Chromium, and any change to Skia rendering 165 Skia's principal downstream user is Chromium, and any change to Skia rendering
161 output can break Chromium. If your change alters rendering in any way, you are 166 output can break Chromium. If your change alters rendering in any way, you are
162 expected to test for and alleviate this. (You may be able to find a Skia team 167 expected to test for and alleviate this. (You may be able to find a Skia team
163 member to help you, but the onus remains on each individual contributor to avoid 168 member to help you, but the onus remains on each individual contributor to avoid
164 breaking Chrome. 169 breaking Chrome.
(...skipping 29 matching lines...) Expand all
194 please ask a Skia committer to land your patch for you or land using the commit 199 please ask a Skia committer to land your patch for you or land using the commit
195 queue. 200 queue.
196 201
197 As part of this process, the Skia committer may create a new codereview 202 As part of this process, the Skia committer may create a new codereview
198 containing your patch (perhaps with some small adjustments at her discretion). 203 containing your patch (perhaps with some small adjustments at her discretion).
199 If so, you can mark your codereview as "Closed", and update it with a link to 204 If so, you can mark your codereview as "Closed", and update it with a link to
200 the new codereview. 205 the new codereview.
201 206
202 ### Skia committers: 207 ### Skia committers:
203 * tips on how to apply the externally provided patch are [here](./patch) 208 * tips on how to apply the externally provided patch are [here](./patch)
204 * when landing externally contributed patches, please note the original 209 * when landing externally contributed patches, please note the original
205 contributor's identity (and provide a link to the original codereview) in t he commit message 210 contributor's identity (and provide a link to the original codereview) in t he commit message
206 211
207 git-cl will squash all your commits into a single one with the description you u sed when you uploaded your change. 212 git-cl will squash all your commits into a single one with the description you u sed when you uploaded your change.
208 213
209 ~~~~ 214 ~~~~
210 git cl land 215 git cl land
211 ~~~~ 216 ~~~~
212 or 217 or
213 ~~~~ 218 ~~~~
214 git cl land -c 'Contributor Name <email@example.com>' 219 git cl land -c 'Contributor Name <email@example.com>'
215 ~~~~ 220 ~~~~
OLDNEW
« no previous file with comments | « no previous file | site/user/download.md » ('j') | site/user/download.md » ('J')

Powered by Google App Engine
This is Rietveld 408576698