1*c8dee2aaSAndroid Build Coastguard Worker--- 2*c8dee2aaSAndroid Build Coastguard Workertitle: 'How to submit a patch' 3*c8dee2aaSAndroid Build Coastguard WorkerlinkTitle: 'How to submit a patch' 4*c8dee2aaSAndroid Build Coastguard Worker--- 5*c8dee2aaSAndroid Build Coastguard Worker 6*c8dee2aaSAndroid Build Coastguard Worker## Configure git 7*c8dee2aaSAndroid Build Coastguard Worker 8*c8dee2aaSAndroid Build Coastguard Worker<!--?prettify lang=sh?--> 9*c8dee2aaSAndroid Build Coastguard Worker 10*c8dee2aaSAndroid Build Coastguard Worker git config --global user.name "Your Name" 11*c8dee2aaSAndroid Build Coastguard Worker git config --global user.email [email protected] 12*c8dee2aaSAndroid Build Coastguard Worker 13*c8dee2aaSAndroid Build Coastguard Worker## Making changes 14*c8dee2aaSAndroid Build Coastguard Worker 15*c8dee2aaSAndroid Build Coastguard WorkerFirst create a branch for your changes: 16*c8dee2aaSAndroid Build Coastguard Worker 17*c8dee2aaSAndroid Build Coastguard Worker<!--?prettify lang=sh?--> 18*c8dee2aaSAndroid Build Coastguard Worker 19*c8dee2aaSAndroid Build Coastguard Worker git config branch.autosetuprebase always 20*c8dee2aaSAndroid Build Coastguard Worker git checkout -b my_feature origin/main 21*c8dee2aaSAndroid Build Coastguard Worker 22*c8dee2aaSAndroid Build Coastguard WorkerAfter making your changes, create a commit 23*c8dee2aaSAndroid Build Coastguard Worker 24*c8dee2aaSAndroid Build Coastguard Worker<!--?prettify lang=sh?--> 25*c8dee2aaSAndroid Build Coastguard Worker 26*c8dee2aaSAndroid Build Coastguard Worker git add [file1] [file2] ... 27*c8dee2aaSAndroid Build Coastguard Worker git commit 28*c8dee2aaSAndroid Build Coastguard Worker 29*c8dee2aaSAndroid Build Coastguard WorkerIf your branch gets out of date, you will need to update it: 30*c8dee2aaSAndroid Build Coastguard Worker 31*c8dee2aaSAndroid Build Coastguard Worker<!--?prettify lang=sh?--> 32*c8dee2aaSAndroid Build Coastguard Worker 33*c8dee2aaSAndroid Build Coastguard Worker git pull 34*c8dee2aaSAndroid Build Coastguard Worker python3 tools/git-sync-deps 35*c8dee2aaSAndroid Build Coastguard Worker 36*c8dee2aaSAndroid Build Coastguard Worker## Adding a unit test 37*c8dee2aaSAndroid Build Coastguard Worker 38*c8dee2aaSAndroid Build Coastguard WorkerIf you are willing to change Skia codebase, it's nice to add a test at the same 39*c8dee2aaSAndroid Build Coastguard Workertime. Skia has a simple unittest framework so you can add a case to it. 40*c8dee2aaSAndroid Build Coastguard Worker 41*c8dee2aaSAndroid Build Coastguard WorkerTest code is located under the 'tests' directory. 42*c8dee2aaSAndroid Build Coastguard Worker 43*c8dee2aaSAndroid Build Coastguard WorkerSee [Writing Unit and Rendering Tests](/docs/dev/testing/tests) for details. 44*c8dee2aaSAndroid Build Coastguard Worker 45*c8dee2aaSAndroid Build Coastguard WorkerUnit tests are best, but if your change touches rendering and you can't think of 46*c8dee2aaSAndroid Build Coastguard Workeran automated way to verify the results, consider writing a GM test. Also, if 47*c8dee2aaSAndroid Build Coastguard Workeryour change is in the GPU code, you may not be able to write it as part of the 48*c8dee2aaSAndroid Build Coastguard Workerstandard unit test suite, but there are GPU-specific testing paths you can 49*c8dee2aaSAndroid Build Coastguard Workerextend. 50*c8dee2aaSAndroid Build Coastguard Worker 51*c8dee2aaSAndroid Build Coastguard Worker## Updating BUILD.bazel files 52*c8dee2aaSAndroid Build Coastguard Worker 53*c8dee2aaSAndroid Build Coastguard WorkerIf you added or removed files, you will need to update the `BUILD.bazel` file in the directory 54*c8dee2aaSAndroid Build Coastguard Workerof those files. Many `BUILD.bazel` files have a list of files that is broken up into two 55*c8dee2aaSAndroid Build Coastguard Worker[`filegroup`](https://bazel.build/reference/be/general#filegroup) rules using the 56*c8dee2aaSAndroid Build Coastguard Worker`split_srcs_and_hdrs` macro. You should add the new file names or delete the old ones from these 57*c8dee2aaSAndroid Build Coastguard Workerfile lists. 58*c8dee2aaSAndroid Build Coastguard Worker 59*c8dee2aaSAndroid Build Coastguard WorkerIf your feature will be conditionally enabled (e.g. like the GPU backends or image codecs), you 60*c8dee2aaSAndroid Build Coastguard Workermay need to add or modify 61*c8dee2aaSAndroid Build Coastguard Worker[`select`](https://bazel.build/reference/be/common-definitions#configurable-attributes) statements 62*c8dee2aaSAndroid Build Coastguard Workerto achieve that goal. Look at existing rules for examples of this. 63*c8dee2aaSAndroid Build Coastguard Worker 64*c8dee2aaSAndroid Build Coastguard Worker## Submitting a patch 65*c8dee2aaSAndroid Build Coastguard Worker 66*c8dee2aaSAndroid Build Coastguard WorkerFor your code to be accepted into the codebase, you must complete the 67*c8dee2aaSAndroid Build Coastguard Worker[Individual Contributor License Agreement](http://code.google.com/legal/individual-cla-v1.0.html). 68*c8dee2aaSAndroid Build Coastguard WorkerYou can do this online, and it only takes a minute. If you are contributing on 69*c8dee2aaSAndroid Build Coastguard Workerbehalf of a corporation, you must fill out the 70*c8dee2aaSAndroid Build Coastguard Worker[Corporate Contributor License Agreement](http://code.google.com/legal/corporate-cla-v1.0.html) 71*c8dee2aaSAndroid Build Coastguard Workerand send it to us as described on that page. Add your (or your organization's) 72*c8dee2aaSAndroid Build Coastguard Workername and contact info to the AUTHORS file as a part of your CL. 73*c8dee2aaSAndroid Build Coastguard Worker 74*c8dee2aaSAndroid Build Coastguard WorkerNow that you've made a change and written a test for it, it's ready for the code 75*c8dee2aaSAndroid Build Coastguard Workerreview! Submit a patch and getting it reviewed is fairly easy with depot tools. 76*c8dee2aaSAndroid Build Coastguard Worker 77*c8dee2aaSAndroid Build Coastguard WorkerUse `git-cl`, which comes with 78*c8dee2aaSAndroid Build Coastguard Worker[depot tools](http://sites.google.com/a/chromium.org/dev/developers/how-tos/install-depot-tools). 79*c8dee2aaSAndroid Build Coastguard WorkerFor help, run `git cl help`. Note that in order for `git cl` to work correctly, 80*c8dee2aaSAndroid Build Coastguard Workerit needs to run on a clone of <https://skia.googlesource.com/skia>. Using clones 81*c8dee2aaSAndroid Build Coastguard Workerof mirrors, including Google's mirror on GitHub, might lead to issues with 82*c8dee2aaSAndroid Build Coastguard Worker`git cl` usage. 83*c8dee2aaSAndroid Build Coastguard Worker 84*c8dee2aaSAndroid Build Coastguard Worker### Find a reviewer 85*c8dee2aaSAndroid Build Coastguard Worker 86*c8dee2aaSAndroid Build Coastguard WorkerIdeally, the reviewer is someone who is familiar with the area of code you are 87*c8dee2aaSAndroid Build Coastguard Workertouching. Look at the git blame for the file to see who else has been editing 88*c8dee2aaSAndroid Build Coastguard Workerit. If unsuccessful, another option is to click on the 'Suggested Reviewers' 89*c8dee2aaSAndroid Build Coastguard Workerbutton to add one of the listed Skia contacts. They should be able to add 90*c8dee2aaSAndroid Build Coastguard Workerappropriate reviewers for your change. The button is located here: 91*c8dee2aaSAndroid Build Coastguard Worker<img src="/docs/dev/contrib/SuggestedReviewers.png" style="display: inline-block; max-width: 75%" /> 92*c8dee2aaSAndroid Build Coastguard Worker 93*c8dee2aaSAndroid Build Coastguard Worker### Uploading changes for review 94*c8dee2aaSAndroid Build Coastguard Worker 95*c8dee2aaSAndroid Build Coastguard WorkerSkia uses the Gerrit code review tool. Skia's instance is 96*c8dee2aaSAndroid Build Coastguard Worker[skia-review](http://skia-review.googlesource.com). Use `git cl` to upload your 97*c8dee2aaSAndroid Build Coastguard Workerchange: 98*c8dee2aaSAndroid Build Coastguard Worker 99*c8dee2aaSAndroid Build Coastguard Worker<!--?prettify lang=sh?--> 100*c8dee2aaSAndroid Build Coastguard Worker 101*c8dee2aaSAndroid Build Coastguard Worker git cl upload 102*c8dee2aaSAndroid Build Coastguard Worker 103*c8dee2aaSAndroid Build Coastguard WorkerYou may have to enter a Google Account username and password to authenticate 104*c8dee2aaSAndroid Build Coastguard Workeryourself to Gerrit. A free gmail account will do fine, or any other type of 105*c8dee2aaSAndroid Build Coastguard WorkerGoogle account. It does not have to match the email address you configured using 106*c8dee2aaSAndroid Build Coastguard Worker`git config --global user.email` above, but it can. 107*c8dee2aaSAndroid Build Coastguard Worker 108*c8dee2aaSAndroid Build Coastguard WorkerThe command output should include a URL, similar to 109*c8dee2aaSAndroid Build Coastguard Worker([https://skia-review.googlesource.com/c/4559/](https://skia-review.googlesource.com/c/4559/)), 110*c8dee2aaSAndroid Build Coastguard Workerindicating where your changelist can be reviewed. 111*c8dee2aaSAndroid Build Coastguard Worker 112*c8dee2aaSAndroid Build Coastguard Worker### Submit try jobs 113*c8dee2aaSAndroid Build Coastguard Worker 114*c8dee2aaSAndroid Build Coastguard WorkerSkia's trybots allow testing and verification of changes before they land in the 115*c8dee2aaSAndroid Build Coastguard Workerrepo. You need to have permission to trigger try jobs; if you need permission, 116*c8dee2aaSAndroid Build Coastguard Workerask a committer. After uploading your CL to 117*c8dee2aaSAndroid Build Coastguard Worker[Gerrit](https://skia-review.googlesource.com/), you may trigger a try job for 118*c8dee2aaSAndroid Build Coastguard Workerany job listed in tasks.json, either via the Gerrit UI, using `git cl try`, eg. 119*c8dee2aaSAndroid Build Coastguard Worker 120*c8dee2aaSAndroid Build Coastguard Worker git cl try -B skia.primary -b Some-Tryjob-Name 121*c8dee2aaSAndroid Build Coastguard Worker 122*c8dee2aaSAndroid Build Coastguard Workeror using bin/try, a small wrapper for `git cl try` which helps to choose try 123*c8dee2aaSAndroid Build Coastguard Workerjobs. From a Skia checkout: 124*c8dee2aaSAndroid Build Coastguard Worker 125*c8dee2aaSAndroid Build Coastguard Worker bin/try --list 126*c8dee2aaSAndroid Build Coastguard Worker 127*c8dee2aaSAndroid Build Coastguard WorkerYou can also search using regular expressions: 128*c8dee2aaSAndroid Build Coastguard Worker 129*c8dee2aaSAndroid Build Coastguard Worker bin/try "Test.*GTX660.*Release" 130*c8dee2aaSAndroid Build Coastguard Worker 131*c8dee2aaSAndroid Build Coastguard WorkerFor more information about testing, see 132*c8dee2aaSAndroid Build Coastguard Worker[testing infrastructure](/docs/dev/testing/automated_testing). 133*c8dee2aaSAndroid Build Coastguard Worker 134*c8dee2aaSAndroid Build Coastguard Worker### Request review 135*c8dee2aaSAndroid Build Coastguard Worker 136*c8dee2aaSAndroid Build Coastguard WorkerGo to the supplied URL or go to the code review page and select the **Your** 137*c8dee2aaSAndroid Build Coastguard Workerdropdown and click on **Changes**. Select the change you want to submit for 138*c8dee2aaSAndroid Build Coastguard Workerreview and click **Reply**. Enter at least one reviewer's email address. Now add 139*c8dee2aaSAndroid Build Coastguard Workerany optional notes, and send your change off for review by clicking on **Send**. 140*c8dee2aaSAndroid Build Coastguard WorkerUnless you send your change to reviewers, no one will know to look at it. 141*c8dee2aaSAndroid Build Coastguard Worker 142*c8dee2aaSAndroid Build Coastguard Worker_Note_: If you don't see editing commands on the review page, click **Sign in** 143*c8dee2aaSAndroid Build Coastguard Workerin the upper right. _Hint_: You can add -r [email protected] --send-mail to 144*c8dee2aaSAndroid Build Coastguard Workersend the email directly when uploading a change using `git-cl`. 145*c8dee2aaSAndroid Build Coastguard Worker 146*c8dee2aaSAndroid Build Coastguard Worker## The review process 147*c8dee2aaSAndroid Build Coastguard Worker 148*c8dee2aaSAndroid Build Coastguard WorkerIf you submit a giant patch, or do a bunch of work without discussing it with 149*c8dee2aaSAndroid Build Coastguard Workerthe relevant people, you may have a hard time convincing anyone to review it! 150*c8dee2aaSAndroid Build Coastguard Worker 151*c8dee2aaSAndroid Build Coastguard WorkerCode reviews are an important part of the engineering process. The reviewer will 152*c8dee2aaSAndroid Build Coastguard Workeralmost always have suggestions or style fixes for you, and it's important not to 153*c8dee2aaSAndroid Build Coastguard Workertake such suggestions personally or as a commentary on your abilities or ideas. 154*c8dee2aaSAndroid Build Coastguard WorkerThis is a process where we work together to make sure that the highest quality 155*c8dee2aaSAndroid Build Coastguard Workercode gets submitted! 156*c8dee2aaSAndroid Build Coastguard Worker 157*c8dee2aaSAndroid Build Coastguard WorkerYou will likely get email back from the reviewer with comments. Fix these and 158*c8dee2aaSAndroid Build Coastguard Workerupdate the patch set in the issue by uploading again. The upload will explain 159*c8dee2aaSAndroid Build Coastguard Workerthat it is updating the current CL and ask you for a message explaining the 160*c8dee2aaSAndroid Build Coastguard Workerchange. Be sure to respond to all comments before you request review of an 161*c8dee2aaSAndroid Build Coastguard Workerupdate. 162*c8dee2aaSAndroid Build Coastguard Worker 163*c8dee2aaSAndroid Build Coastguard WorkerIf you need to update code the code on an already uploaded CL, simply edit the 164*c8dee2aaSAndroid Build Coastguard Workercode, commit it again locally, and then run git cl upload again e.g. 165*c8dee2aaSAndroid Build Coastguard Worker 166*c8dee2aaSAndroid Build Coastguard Worker echo "GOATS" > whitespace.txt 167*c8dee2aaSAndroid Build Coastguard Worker git add whitespace.txt 168*c8dee2aaSAndroid Build Coastguard Worker git commit -m 'add GOATS fix to whitespace.txt' 169*c8dee2aaSAndroid Build Coastguard Worker git cl upload 170*c8dee2aaSAndroid Build Coastguard Worker 171*c8dee2aaSAndroid Build Coastguard WorkerOnce you're ready for another review, use **Reply** again to send another 172*c8dee2aaSAndroid Build Coastguard Workernotification (it is helpful to tell the reviewer what you did with respect to 173*c8dee2aaSAndroid Build Coastguard Workereach of their comments). When the reviewer is happy with your patch, they will 174*c8dee2aaSAndroid Build Coastguard Workerapprove your change by setting the Code-Review label to "+1". 175*c8dee2aaSAndroid Build Coastguard Worker 176*c8dee2aaSAndroid Build Coastguard Worker_Note_: As you work through the review process, both you and your reviewers 177*c8dee2aaSAndroid Build Coastguard Workershould converse using the code review interface, and send notes. 178*c8dee2aaSAndroid Build Coastguard Worker 179*c8dee2aaSAndroid Build Coastguard WorkerOnce your change has received an approval, you can click the "Submit to CQ" 180*c8dee2aaSAndroid Build Coastguard Workerbutton on the codereview page and it will be committed on your behalf. 181*c8dee2aaSAndroid Build Coastguard Worker 182*c8dee2aaSAndroid Build Coastguard WorkerOnce your commit has gone in, you should delete the branch containing your 183*c8dee2aaSAndroid Build Coastguard Workerchange: 184*c8dee2aaSAndroid Build Coastguard Worker 185*c8dee2aaSAndroid Build Coastguard Worker git checkout -q origin/main 186*c8dee2aaSAndroid Build Coastguard Worker git branch -D my_feature 187*c8dee2aaSAndroid Build Coastguard Worker 188*c8dee2aaSAndroid Build Coastguard Worker## Final Testing 189*c8dee2aaSAndroid Build Coastguard Worker 190*c8dee2aaSAndroid Build Coastguard WorkerSkia's principal downstream user is Chromium, and any change to Skia rendering 191*c8dee2aaSAndroid Build Coastguard Workeroutput can break Chromium. If your change alters rendering in any way, you are 192*c8dee2aaSAndroid Build Coastguard Workerexpected to test for and alleviate this. You may be able to find a Skia team 193*c8dee2aaSAndroid Build Coastguard Workermember to help you, but the onus remains on each individual contributor to avoid 194*c8dee2aaSAndroid Build Coastguard Workerbreaking Chrome. 195*c8dee2aaSAndroid Build Coastguard Worker 196*c8dee2aaSAndroid Build Coastguard Worker### Evaluating Impact on Chromium 197*c8dee2aaSAndroid Build Coastguard Worker 198*c8dee2aaSAndroid Build Coastguard WorkerKeep in mind that Skia is rolled daily into Blink and Chromium. Run local tests 199*c8dee2aaSAndroid Build Coastguard Workerand watch canary bots for results to ensure no impact. If you are submitting 200*c8dee2aaSAndroid Build Coastguard Workerchanges that will impact layout tests, follow the guides below and/or work with 201*c8dee2aaSAndroid Build Coastguard Workeryour friendly Skia-Blink engineer to evaluate, rebaseline, and land your 202*c8dee2aaSAndroid Build Coastguard Workerchanges. 203*c8dee2aaSAndroid Build Coastguard Worker 204*c8dee2aaSAndroid Build Coastguard WorkerResources: 205*c8dee2aaSAndroid Build Coastguard Worker 206*c8dee2aaSAndroid Build Coastguard Worker[How to land Skia changes that change Blink layout test results](/docs/dev/chrome/blink/) 207*c8dee2aaSAndroid Build Coastguard Worker 208*c8dee2aaSAndroid Build Coastguard WorkerIf you're changing the Skia API, you may need to make an associated change in 209*c8dee2aaSAndroid Build Coastguard WorkerChromium. If you do, please follow these instructions: 210*c8dee2aaSAndroid Build Coastguard Worker[Landing Skia changes which require Chrome changes](/docs/dev/chrome/changes/) 211*c8dee2aaSAndroid Build Coastguard Worker 212*c8dee2aaSAndroid Build Coastguard Worker## Check in your changes 213*c8dee2aaSAndroid Build Coastguard Worker 214*c8dee2aaSAndroid Build Coastguard Worker### Non-Skia-committers 215*c8dee2aaSAndroid Build Coastguard Worker 216*c8dee2aaSAndroid Build Coastguard WorkerIf you already have committer rights, you can follow the directions below to 217*c8dee2aaSAndroid Build Coastguard Workercommit your change directly to Skia's repository. 218*c8dee2aaSAndroid Build Coastguard Worker 219*c8dee2aaSAndroid Build Coastguard WorkerIf you don't have committer rights in https://skia.googlesource.com/skia.git ... 220*c8dee2aaSAndroid Build Coastguard Workerfirst of all, thanks for submitting your patch! We really appreciate these 221*c8dee2aaSAndroid Build Coastguard Workersubmissions. After receiving an approval from a committer, you will be able to 222*c8dee2aaSAndroid Build Coastguard Workerclick the "Submit to CQ" button and submit your patch via the commit queue. 223*c8dee2aaSAndroid Build Coastguard Worker 224*c8dee2aaSAndroid Build Coastguard WorkerIn special instances, a Skia committer may assist you in landing the change by 225*c8dee2aaSAndroid Build Coastguard Workeruploading a new codereview containing your patch (perhaps with some small 226*c8dee2aaSAndroid Build Coastguard Workeradjustments at their discretion). If so, you can mark your change as 227*c8dee2aaSAndroid Build Coastguard Worker"Abandoned", and update it with a link to the new codereview. 228*c8dee2aaSAndroid Build Coastguard Worker 229*c8dee2aaSAndroid Build Coastguard Worker### Skia committers 230*c8dee2aaSAndroid Build Coastguard Worker 231*c8dee2aaSAndroid Build Coastguard Worker- tips on how to apply an externally provided patch are [here](../patch) 232*c8dee2aaSAndroid Build Coastguard Worker- when landing externally contributed patches, please note the original 233*c8dee2aaSAndroid Build Coastguard Worker contributor's identity (and provide a link to the original codereview) in the 234*c8dee2aaSAndroid Build Coastguard Worker commit message 235*c8dee2aaSAndroid Build Coastguard Worker 236*c8dee2aaSAndroid Build Coastguard Worker `git-cl` will squash all your commits into a single one with the description 237*c8dee2aaSAndroid Build Coastguard Worker you used when you uploaded your change. 238*c8dee2aaSAndroid Build Coastguard Worker 239*c8dee2aaSAndroid Build Coastguard Worker ``` 240*c8dee2aaSAndroid Build Coastguard Worker git cl land 241*c8dee2aaSAndroid Build Coastguard Worker ``` 242*c8dee2aaSAndroid Build Coastguard Worker 243*c8dee2aaSAndroid Build Coastguard Worker or 244*c8dee2aaSAndroid Build Coastguard Worker 245*c8dee2aaSAndroid Build Coastguard Worker ``` 246*c8dee2aaSAndroid Build Coastguard Worker git cl land -c 'Contributor Name <[email protected]>' 247*c8dee2aaSAndroid Build Coastguard Worker ``` 248