| 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 |