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 $ 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 Loading... | |
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 Loading... | |
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 ~~~~ |
OLD | NEW |