OLD | NEW |
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 <!--?prettify lang=sh?--> |
11 $ git checkout -b my_feature origin/master | 19 |
12 ~~~~ | 20 git config branch.autosetuprebase always |
| 21 git checkout -b my_feature origin/master |
13 | 22 |
14 After making your changes, create a commit | 23 After making your changes, create a commit |
15 | 24 |
16 ~~~~ | 25 <!--?prettify lang=sh?--> |
17 $ git add [file1] [file2] ... | 26 |
18 $ git commit | 27 git add [file1] [file2] ... |
19 ~~~~ | 28 git commit |
20 | 29 |
21 If your branch gets out of date, you will need to update it: | 30 If your branch gets out of date, you will need to update it: |
22 | 31 |
23 ~~~~ | 32 <!--?prettify lang=sh?--> |
24 $ git pull --rebase | |
25 $ gclient sync | |
26 ~~~~ | |
27 | 33 |
| 34 git pull |
| 35 python bin/sync-and-gyp |
| 36 |
| 37 <!-- |
| 38 python tools/git-sync-deps |
| 39 python ./gyp_skia |
| 40 --> |
28 | 41 |
29 Adding a unit test | 42 Adding a unit test |
30 ------------------ | 43 ------------------ |
31 | 44 |
32 If you are willing to change Skia codebase, it's nice to add a test at the same | 45 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. | 46 time. Skia has a simple unittest framework so you can add a case to it. |
34 | 47 |
35 Test code is located under the 'tests' directory. | 48 Test code is located under the 'tests' directory. |
36 | 49 |
37 See [Writing Unit and Rendering Tests](tests) for details. | 50 See [Writing Unit and Rendering Tests](../testing/tests) for details. |
38 | 51 |
39 Unit tests are best, but if your change touches rendering and you can't think of | 52 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 | 53 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 | 54 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 | 55 it as part of the standard unit test suite, but there are GPU-specific testing |
43 paths you can extend. | 56 paths you can extend. |
44 | 57 |
45 Submitting a patch | 58 Submitting a patch |
46 ------------------ | 59 ------------------ |
47 | 60 |
48 For your code to be accepted into the codebase, you must complete the | 61 For your code to be accepted into the codebase, you must complete the |
49 [Individual Contributor License | 62 [Individual Contributor License |
50 Agreement](http://code.google.com/legal/individual-cla-v1.0.html). You can do | 63 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 | 64 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) | 65 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) | 66 Agreement](http://code.google.com/legal/corporate-cla-v1.0.html) |
| 67 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. | 68 name and contact info to the AUTHORS file as a part of your CL. |
55 | 69 |
56 Now that you've made a change and written a test for it, it's ready for the code | 70 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. | 71 review! Submit a patch and getting it reviewed is fairly easy with depot tools. |
58 | 72 |
59 Use git-cl, which comes with [depot tools](http://sites.google.com/a/chromium.or
g/dev/developers/how-tos/install-depot-tools). | 73 Use git-cl, which comes with [depot |
| 74 tools](http://sites.google.com/a/chromium.org/dev/developers/how-tos/install-dep
ot-tools). |
60 For help, run git-cl help. | 75 For help, run git-cl help. |
61 | 76 |
62 ### Configuring git-cl | 77 ### Configuring git-cl |
63 | 78 |
64 Before using any git-cl commands you will need to configure it to point at the | 79 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: | 80 correct code review server. This is accomplished with the following command: |
66 | 81 |
67 ~~~~ | 82 <!--?prettify lang=sh?--> |
68 git cl config https://skia.googlesource.com/skia/+/master/codereview.settings | 83 |
69 ~~~~ | 84 git cl config https://skia.googlesource.com/skia/+/master/codereview.setting
s |
70 | 85 |
71 ### Find a reviewer | 86 ### Find a reviewer |
72 | 87 |
73 Ideally, the reviewer is someone who is familiar with the area of code you are | 88 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 | 89 touching. If you have doubts, look at the git blame for the file to see who else |
75 has been editing it. | 90 has been editing it. |
76 | 91 |
77 ### Uploading changes for review | 92 ### Uploading changes for review |
78 | 93 |
79 Skia uses Chromium's code review [site](http://codereview.chromium.org) and the | 94 Skia uses Chromium's code review [site](http://codereview.chromium.org) and the |
80 Rietveld open source code review tool. | 95 Rietveld open source code review tool. |
81 Use git cl to upload your change: | 96 Use git cl to upload your change: |
82 ~~~~ | 97 |
83 $ git cl upload | 98 <!--?prettify lang=sh?--> |
84 ~~~~ | 99 |
| 100 git cl upload |
85 | 101 |
86 You may have to enter a Google Account username and password to authenticate | 102 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 | 103 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 | 104 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. | 105 configured using `git config --global user.email` above, but it can. |
90 | 106 |
91 The command output should include a URL, similar to | 107 The command output should include a URL, similar to |
92 (https://codereview.chromium.org/111893004/), indicating where your changelist | 108 (https://codereview.chromium.org/111893004/), indicating where your changelist |
93 can be reviewed. | 109 can be reviewed. |
94 | 110 |
95 ### Request review | 111 ### Request review |
96 | 112 |
97 Go to the supplied URL or go to the code review page and click **Issues created | 113 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 | 114 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**. | 115 Issue**. Enter at least one reviewer's email address and click **Update Issue**. |
(...skipping 23 matching lines...) Expand all Loading... |
123 | 139 |
124 You will likely get email back from the reviewer with comments. Fix these and | 140 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 | 141 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 | 142 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 | 143 change. Be sure to respond to all comments before you request review of an |
128 update. | 144 update. |
129 | 145 |
130 If you need to update code the code on an already uploaded CL, simply edit the | 146 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. | 147 code, commit it again locally, and then run git cl upload again e.g. |
132 | 148 |
133 ~~~~ | 149 echo "GOATS" > whitespace.txt |
134 echo "" > GOATS | 150 git add whitespace.txt |
135 git add GOATS | 151 git commit -m 'add GOATS fix to whitespace.txt' |
136 git commit -m 'add newline fix to GOATS' | 152 git cl upload |
137 git cl upload | |
138 ~~~~ | |
139 | 153 |
140 Once you're ready for another review, use **Publish+Mail Comments** again to | 154 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 | 155 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, | 156 respect to each of their comments). When the reviewer is happy with your patch, |
143 they will say "LGTM" ("Looks Good To Me"). | 157 they will say "LGTM" ("Looks Good To Me"). |
144 | 158 |
145 _Note_: As you work through the review process, both you and your reviewers | 159 _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 | 160 should converse using the code review interface, and send notes using |
147 **Publish+Mail Comments**. | 161 **Publish+Mail Comments**. |
148 | 162 |
| 163 Once your change has received an LGTM, you can check the "Commit" box |
| 164 on the codereview page and it will be committed on your behalf. |
| 165 |
149 Once your commit has gone in, you should delete the branch containing your chang
e: | 166 Once your commit has gone in, you should delete the branch containing your chang
e: |
150 | 167 |
151 ~~~~ | 168 git checkout -q origin/master |
152 $ git checkout master | 169 git branch -D my_feature |
153 $ git branch -D my_feature | |
154 ~~~~ | |
155 | 170 |
156 | 171 |
157 Final Testing | 172 Final Testing |
158 ------------- | 173 ------------- |
159 | 174 |
160 Skia's principal downstream user is Chromium, and any change to Skia rendering | 175 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 | 176 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 | 177 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 | 178 member to help you, but the onus remains on each individual contributor to avoid |
164 breaking Chrome. | 179 breaking Chrome. |
(...skipping 29 matching lines...) Expand all Loading... |
194 please ask a Skia committer to land your patch for you or land using the commit | 209 please ask a Skia committer to land your patch for you or land using the commit |
195 queue. | 210 queue. |
196 | 211 |
197 As part of this process, the Skia committer may create a new codereview | 212 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). | 213 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 | 214 If so, you can mark your codereview as "Closed", and update it with a link to |
200 the new codereview. | 215 the new codereview. |
201 | 216 |
202 ### Skia committers: | 217 ### Skia committers: |
203 * tips on how to apply the externally provided patch are [here](./patch) | 218 * tips on how to apply the externally provided patch are [here](./patch) |
204 * when landing externally contributed patches, please note the original | 219 * 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 | 220 contributor's identity (and provide a link to the original codereview) in t
he commit message |
206 | 221 |
207 git-cl will squash all your commits into a single one with the description you u
sed when you uploaded your change. | 222 git-cl will squash all your commits into a single one with the description you u
sed when you uploaded your change. |
208 | 223 |
209 ~~~~ | 224 ~~~~ |
210 git cl land | 225 git cl land |
211 ~~~~ | 226 ~~~~ |
212 or | 227 or |
213 ~~~~ | 228 ~~~~ |
214 git cl land -c 'Contributor Name <email@example.com>' | 229 git cl land -c 'Contributor Name <email@example.com>' |
215 ~~~~ | 230 ~~~~ |
OLD | NEW |