1 How to submit a patch 2 ===================== 3 4 5 Making changes 6 -------------- 7 8 First create a branch for your changes: 9 10 ~~~~ 11 $ git checkout -b my_feature origin/master 12 ~~~~ 13 14 After making your changes, create a commit 15 16 ~~~~ 17 $ git add [file1] [file2] ... 18 $ git commit 19 ~~~~ 20 21 If your branch gets out of date, you will need to update it: 22 23 ~~~~ 24 $ git pull --rebase 25 $ gclient sync 26 ~~~~ 27 28 29 Adding a unit test 30 ------------------ 31 32 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. 34 35 Test code is located under the 'tests' directory. 36 37 See [Writing Unit and Rendering Tests](tests) for details. 38 39 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 41 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 43 paths you can extend. 44 45 Submitting a patch 46 ------------------ 47 48 For your code to be accepted into the codebase, you must complete the 49 [Individual Contributor License 50 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 52 corporation, you must fill out the [Corporate Contributor License Agreement](http://code.google.com/legal/corporate-cla-v1.0.html) 53 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. 55 56 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. 58 59 Use git-cl, which comes with [depot tools](http://sites.google.com/a/chromium.org/dev/developers/how-tos/install-depot-tools). 60 For help, run git-cl help. 61 62 ### Configuring git-cl 63 64 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: 66 67 ~~~~ 68 git cl config https://skia.googlesource.com/skia/+/master/codereview.settings 69 ~~~~ 70 71 ### Find a reviewer 72 73 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 75 has been editing it. 76 77 ### Uploading changes for review 78 79 Skia uses Chromium's code review [site](http://codereview.chromium.org) and the 80 Rietveld open source code review tool. 81 Use git cl to upload your change: 82 ~~~~ 83 $ git cl upload 84 ~~~~ 85 86 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 88 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. 90 91 The command output should include a URL, similar to 92 (https://codereview.chromium.org/111893004/), indicating where your changelist 93 can be reviewed. 94 95 ### Request review 96 97 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 99 Issue**. Enter at least one reviewer's email address and click **Update Issue**. 100 Now click on **Publish+Mail Comments**, add any optional notes, and send your 101 change off for review. Unless you publish your change, no one will know to look 102 at it. 103 104 _Note_: If you don't see editing commands on the review page, click **Log In** 105 in the upper right. _Hint_: You can add -r reviewer (a] example.com --send-mail to 106 send the email directly when uploading a change in both gcl and git-cl. 107 108 109 The review process 110 ------------------ 111 112 If you submit a giant patch, or do a bunch of work without discussing it with 113 the relevant people, you may have a hard time convincing anyone to review it! 114 115 Please follow the guidelines on how to conduct a code review detailed here: 116 https://code.google.com/p/rietveld/wiki/CodeReviewHelp 117 118 Code reviews are an important part of the engineering process. The reviewer will 119 almost always have suggestions or style fixes for you, and it's important not to 120 take such suggestions personally or as a commentary on your abilities or ideas. 121 This is a process where we work together to make sure that the highest quality 122 code gets submitted! 123 124 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 126 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 128 update. 129 130 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. 132 133 ~~~~ 134 echo "" > GOATS 135 git add GOATS 136 git commit -m 'add newline fix to GOATS' 137 git cl upload 138 ~~~~ 139 140 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 142 respect to each of their comments). When the reviewer is happy with your patch, 143 they will say "LGTM" ("Looks Good To Me"). 144 145 _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 147 **Publish+Mail Comments**. 148 149 Once your commit has gone in, you should delete the branch containing your change: 150 151 ~~~~ 152 $ git checkout master 153 $ git branch -D my_feature 154 ~~~~ 155 156 157 Final Testing 158 ------------- 159 160 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 162 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 164 breaking Chrome. 165 166 ### Evaluating Impact on Chromium 167 168 Keep in mind that Skia is rolled daily into Blink and Chromium. Run local tests 169 and watch canary bots for results to ensure no impact. If you are submitting 170 changes that will impact layout tests, follow the guides below and/or work with 171 your friendly Skia-Blink engineer to evaluate, rebaseline, and land your 172 changes. 173 174 Resources: 175 176 [How to land Skia changes that change Blink layout test results](../chrome/layouttest) 177 178 If you're changing the Skia API, you may need to make an associated change in Chromium. 179 If you do, please follow these instructions: [Landing Skia changes which require Chrome changes](../chrome/changes) 180 181 182 Check in your changes 183 --------------------- 184 185 ### Non-Skia-committers 186 187 If you already have committer rights, you can follow the directions below to 188 commit your change directly to Skia's repository. 189 190 If you don't have committer rights in https://skia.googlesource.com/skia.git ... 191 first of all, thanks for submitting your patch! We really appreciate these 192 submissions. Unfortunately, we don't yet have a way for Skia committers to mark 193 a patch as "approved" and thus allow non-committers to commit them. So instead, 194 please ask a Skia committer to land your patch for you or land using the commit 195 queue. 196 197 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). 199 If so, you can mark your codereview as "Closed", and update it with a link to 200 the new codereview. 201 202 ### Skia committers: 203 * tips on how to apply the externally provided patch are [here](./patch) 204 * when landing externally contributed patches, please note the original 205 contributor's identity (and provide a link to the original codereview) in the commit message 206 207 git-cl will squash all your commits into a single one with the description you used when you uploaded your change. 208 209 ~~~~ 210 git cl land 211 ~~~~ 212 or 213 ~~~~ 214 git cl land -c 'Contributor Name <email (a] example.com>' 215 ~~~~ 216
