1cb93a386Sopenharmony_ci--- 2cb93a386Sopenharmony_cititle: 'How to submit a patch' 3cb93a386Sopenharmony_cilinkTitle: 'How to submit a patch' 4cb93a386Sopenharmony_ci--- 5cb93a386Sopenharmony_ci 6cb93a386Sopenharmony_ci## Configure git 7cb93a386Sopenharmony_ci 8cb93a386Sopenharmony_ci<!--?prettify lang=sh?--> 9cb93a386Sopenharmony_ci 10cb93a386Sopenharmony_ci git config --global user.name "Your Name" 11cb93a386Sopenharmony_ci git config --global user.email you@example.com 12cb93a386Sopenharmony_ci 13cb93a386Sopenharmony_ci## Making changes 14cb93a386Sopenharmony_ci 15cb93a386Sopenharmony_ciFirst create a branch for your changes: 16cb93a386Sopenharmony_ci 17cb93a386Sopenharmony_ci<!--?prettify lang=sh?--> 18cb93a386Sopenharmony_ci 19cb93a386Sopenharmony_ci git config branch.autosetuprebase always 20cb93a386Sopenharmony_ci git checkout -b my_feature origin/main 21cb93a386Sopenharmony_ci 22cb93a386Sopenharmony_ciAfter making your changes, create a commit 23cb93a386Sopenharmony_ci 24cb93a386Sopenharmony_ci<!--?prettify lang=sh?--> 25cb93a386Sopenharmony_ci 26cb93a386Sopenharmony_ci git add [file1] [file2] ... 27cb93a386Sopenharmony_ci git commit 28cb93a386Sopenharmony_ci 29cb93a386Sopenharmony_ciIf your branch gets out of date, you will need to update it: 30cb93a386Sopenharmony_ci 31cb93a386Sopenharmony_ci<!--?prettify lang=sh?--> 32cb93a386Sopenharmony_ci 33cb93a386Sopenharmony_ci git pull 34cb93a386Sopenharmony_ci python2 tools/git-sync-deps 35cb93a386Sopenharmony_ci 36cb93a386Sopenharmony_ci## Adding a unit test 37cb93a386Sopenharmony_ci 38cb93a386Sopenharmony_ciIf you are willing to change Skia codebase, it's nice to add a test at the same 39cb93a386Sopenharmony_citime. Skia has a simple unittest framework so you can add a case to it. 40cb93a386Sopenharmony_ci 41cb93a386Sopenharmony_ciTest code is located under the 'tests' directory. 42cb93a386Sopenharmony_ci 43cb93a386Sopenharmony_ciSee [Writing Unit and Rendering Tests](/docs/dev/testing/tests) for details. 44cb93a386Sopenharmony_ci 45cb93a386Sopenharmony_ciUnit tests are best, but if your change touches rendering and you can't think of 46cb93a386Sopenharmony_cian automated way to verify the results, consider writing a GM test. Also, if 47cb93a386Sopenharmony_ciyour change is in the GPU code, you may not be able to write it as part of the 48cb93a386Sopenharmony_cistandard unit test suite, but there are GPU-specific testing paths you can 49cb93a386Sopenharmony_ciextend. 50cb93a386Sopenharmony_ci 51cb93a386Sopenharmony_ci## Submitting a patch 52cb93a386Sopenharmony_ci 53cb93a386Sopenharmony_ciFor your code to be accepted into the codebase, you must complete the 54cb93a386Sopenharmony_ci[Individual Contributor License Agreement](http://code.google.com/legal/individual-cla-v1.0.html). 55cb93a386Sopenharmony_ciYou can do this online, and it only takes a minute. If you are contributing on 56cb93a386Sopenharmony_cibehalf of a corporation, you must fill out the 57cb93a386Sopenharmony_ci[Corporate Contributor License Agreement](http://code.google.com/legal/corporate-cla-v1.0.html) 58cb93a386Sopenharmony_ciand send it to us as described on that page. Add your (or your organization's) 59cb93a386Sopenharmony_ciname and contact info to the AUTHORS file as a part of your CL. 60cb93a386Sopenharmony_ci 61cb93a386Sopenharmony_ciNow that you've made a change and written a test for it, it's ready for the code 62cb93a386Sopenharmony_cireview! Submit a patch and getting it reviewed is fairly easy with depot tools. 63cb93a386Sopenharmony_ci 64cb93a386Sopenharmony_ciUse `git-cl`, which comes with 65cb93a386Sopenharmony_ci[depot tools](http://sites.google.com/a/chromium.org/dev/developers/how-tos/install-depot-tools). 66cb93a386Sopenharmony_ciFor help, run `git cl help`. Note that in order for `git cl` to work correctly, 67cb93a386Sopenharmony_ciit needs to run on a clone of <https://skia.googlesource.com/skia>. Using clones 68cb93a386Sopenharmony_ciof mirrors, including Google's mirror on GitHub, might lead to issues with 69cb93a386Sopenharmony_ci`git cl` usage. 70cb93a386Sopenharmony_ci 71cb93a386Sopenharmony_ci### Find a reviewer 72cb93a386Sopenharmony_ci 73cb93a386Sopenharmony_ciIdeally, the reviewer is someone who is familiar with the area of code you are 74cb93a386Sopenharmony_citouching. Look at the git blame for the file to see who else has been editing 75cb93a386Sopenharmony_ciit. If unsuccessful, another option is to click on the 'Suggested Reviewers' 76cb93a386Sopenharmony_cibutton to add one of the listed Skia contacts. They should be able to add 77cb93a386Sopenharmony_ciappropriate reviewers for your change. The button is located here: 78cb93a386Sopenharmony_ci<img src="/docs/dev/contrib/SuggestedReviewers.png" style="display: inline-block; max-width: 75%" /> 79cb93a386Sopenharmony_ci 80cb93a386Sopenharmony_ci### Uploading changes for review 81cb93a386Sopenharmony_ci 82cb93a386Sopenharmony_ciSkia uses the Gerrit code review tool. Skia's instance is 83cb93a386Sopenharmony_ci[skia-review](http://skia-review.googlesource.com). Use `git cl` to upload your 84cb93a386Sopenharmony_cichange: 85cb93a386Sopenharmony_ci 86cb93a386Sopenharmony_ci<!--?prettify lang=sh?--> 87cb93a386Sopenharmony_ci 88cb93a386Sopenharmony_ci git cl upload 89cb93a386Sopenharmony_ci 90cb93a386Sopenharmony_ciYou may have to enter a Google Account username and password to authenticate 91cb93a386Sopenharmony_ciyourself to Gerrit. A free gmail account will do fine, or any other type of 92cb93a386Sopenharmony_ciGoogle account. It does not have to match the email address you configured using 93cb93a386Sopenharmony_ci`git config --global user.email` above, but it can. 94cb93a386Sopenharmony_ci 95cb93a386Sopenharmony_ciThe command output should include a URL, similar to 96cb93a386Sopenharmony_ci(https://skia-review.googlesource.com/c/4559/), indicating where your changelist 97cb93a386Sopenharmony_cican be reviewed. 98cb93a386Sopenharmony_ci 99cb93a386Sopenharmony_ci### Submit try jobs 100cb93a386Sopenharmony_ci 101cb93a386Sopenharmony_ciSkia's trybots allow testing and verification of changes before they land in the 102cb93a386Sopenharmony_cirepo. You need to have permission to trigger try jobs; if you need permission, 103cb93a386Sopenharmony_ciask a committer. After uploading your CL to 104cb93a386Sopenharmony_ci[Gerrit](https://skia-review.googlesource.com/), you may trigger a try job for 105cb93a386Sopenharmony_ciany job listed in tasks.json, either via the Gerrit UI, using `git cl try`, eg. 106cb93a386Sopenharmony_ci 107cb93a386Sopenharmony_ci git cl try -B skia.primary -b Some-Tryjob-Name 108cb93a386Sopenharmony_ci 109cb93a386Sopenharmony_cior using bin/try, a small wrapper for `git cl try` which helps to choose try 110cb93a386Sopenharmony_cijobs. From a Skia checkout: 111cb93a386Sopenharmony_ci 112cb93a386Sopenharmony_ci bin/try --list 113cb93a386Sopenharmony_ci 114cb93a386Sopenharmony_ciYou can also search using regular expressions: 115cb93a386Sopenharmony_ci 116cb93a386Sopenharmony_ci bin/try "Test.*GTX660.*Release" 117cb93a386Sopenharmony_ci 118cb93a386Sopenharmony_ciFor more information about testing, see 119cb93a386Sopenharmony_ci[testing infrastructure](/docs/dev/testing/automated_testing). 120cb93a386Sopenharmony_ci 121cb93a386Sopenharmony_ci### Request review 122cb93a386Sopenharmony_ci 123cb93a386Sopenharmony_ciGo to the supplied URL or go to the code review page and select the **Your** 124cb93a386Sopenharmony_cidropdown and click on **Changes**. Select the change you want to submit for 125cb93a386Sopenharmony_cireview and click **Reply**. Enter at least one reviewer's email address. Now add 126cb93a386Sopenharmony_ciany optional notes, and send your change off for review by clicking on **Send**. 127cb93a386Sopenharmony_ciUnless you send your change to reviewers, no one will know to look at it. 128cb93a386Sopenharmony_ci 129cb93a386Sopenharmony_ci_Note_: If you don't see editing commands on the review page, click **Sign in** 130cb93a386Sopenharmony_ciin the upper right. _Hint_: You can add -r reviewer@example.com --send-mail to 131cb93a386Sopenharmony_cisend the email directly when uploading a change using `git-cl`. 132cb93a386Sopenharmony_ci 133cb93a386Sopenharmony_ci## The review process 134cb93a386Sopenharmony_ci 135cb93a386Sopenharmony_ciIf you submit a giant patch, or do a bunch of work without discussing it with 136cb93a386Sopenharmony_cithe relevant people, you may have a hard time convincing anyone to review it! 137cb93a386Sopenharmony_ci 138cb93a386Sopenharmony_ciCode reviews are an important part of the engineering process. The reviewer will 139cb93a386Sopenharmony_cialmost always have suggestions or style fixes for you, and it's important not to 140cb93a386Sopenharmony_citake such suggestions personally or as a commentary on your abilities or ideas. 141cb93a386Sopenharmony_ciThis is a process where we work together to make sure that the highest quality 142cb93a386Sopenharmony_cicode gets submitted! 143cb93a386Sopenharmony_ci 144cb93a386Sopenharmony_ciYou will likely get email back from the reviewer with comments. Fix these and 145cb93a386Sopenharmony_ciupdate the patch set in the issue by uploading again. The upload will explain 146cb93a386Sopenharmony_cithat it is updating the current CL and ask you for a message explaining the 147cb93a386Sopenharmony_cichange. Be sure to respond to all comments before you request review of an 148cb93a386Sopenharmony_ciupdate. 149cb93a386Sopenharmony_ci 150cb93a386Sopenharmony_ciIf you need to update code the code on an already uploaded CL, simply edit the 151cb93a386Sopenharmony_cicode, commit it again locally, and then run git cl upload again e.g. 152cb93a386Sopenharmony_ci 153cb93a386Sopenharmony_ci echo "GOATS" > whitespace.txt 154cb93a386Sopenharmony_ci git add whitespace.txt 155cb93a386Sopenharmony_ci git commit -m 'add GOATS fix to whitespace.txt' 156cb93a386Sopenharmony_ci git cl upload 157cb93a386Sopenharmony_ci 158cb93a386Sopenharmony_ciOnce you're ready for another review, use **Reply** again to send another 159cb93a386Sopenharmony_cinotification (it is helpful to tell the reviewer what you did with respect to 160cb93a386Sopenharmony_cieach of their comments). When the reviewer is happy with your patch, they will 161cb93a386Sopenharmony_ciapprove your change by setting the Code-Review label to "+1". 162cb93a386Sopenharmony_ci 163cb93a386Sopenharmony_ci_Note_: As you work through the review process, both you and your reviewers 164cb93a386Sopenharmony_cishould converse using the code review interface, and send notes. 165cb93a386Sopenharmony_ci 166cb93a386Sopenharmony_ciOnce your change has received an approval, you can click the "Submit to CQ" 167cb93a386Sopenharmony_cibutton on the codereview page and it will be committed on your behalf. 168cb93a386Sopenharmony_ci 169cb93a386Sopenharmony_ciOnce your commit has gone in, you should delete the branch containing your 170cb93a386Sopenharmony_cichange: 171cb93a386Sopenharmony_ci 172cb93a386Sopenharmony_ci git checkout -q origin/main 173cb93a386Sopenharmony_ci git branch -D my_feature 174cb93a386Sopenharmony_ci 175cb93a386Sopenharmony_ci## Final Testing 176cb93a386Sopenharmony_ci 177cb93a386Sopenharmony_ciSkia's principal downstream user is Chromium, and any change to Skia rendering 178cb93a386Sopenharmony_cioutput can break Chromium. If your change alters rendering in any way, you are 179cb93a386Sopenharmony_ciexpected to test for and alleviate this. You may be able to find a Skia team 180cb93a386Sopenharmony_cimember to help you, but the onus remains on each individual contributor to avoid 181cb93a386Sopenharmony_cibreaking Chrome. 182cb93a386Sopenharmony_ci 183cb93a386Sopenharmony_ci### Evaluating Impact on Chromium 184cb93a386Sopenharmony_ci 185cb93a386Sopenharmony_ciKeep in mind that Skia is rolled daily into Blink and Chromium. Run local tests 186cb93a386Sopenharmony_ciand watch canary bots for results to ensure no impact. If you are submitting 187cb93a386Sopenharmony_cichanges that will impact layout tests, follow the guides below and/or work with 188cb93a386Sopenharmony_ciyour friendly Skia-Blink engineer to evaluate, rebaseline, and land your 189cb93a386Sopenharmony_cichanges. 190cb93a386Sopenharmony_ci 191cb93a386Sopenharmony_ciResources: 192cb93a386Sopenharmony_ci 193cb93a386Sopenharmony_ci[How to land Skia changes that change Blink layout test results](/docs/dev/chrome/blink/) 194cb93a386Sopenharmony_ci 195cb93a386Sopenharmony_ciIf you're changing the Skia API, you may need to make an associated change in 196cb93a386Sopenharmony_ciChromium. If you do, please follow these instructions: 197cb93a386Sopenharmony_ci[Landing Skia changes which require Chrome changes](/docs/dev/chrome/changes/) 198cb93a386Sopenharmony_ci 199cb93a386Sopenharmony_ci## Check in your changes 200cb93a386Sopenharmony_ci 201cb93a386Sopenharmony_ci### Non-Skia-committers 202cb93a386Sopenharmony_ci 203cb93a386Sopenharmony_ciIf you already have committer rights, you can follow the directions below to 204cb93a386Sopenharmony_cicommit your change directly to Skia's repository. 205cb93a386Sopenharmony_ci 206cb93a386Sopenharmony_ciIf you don't have committer rights in https://skia.googlesource.com/skia.git ... 207cb93a386Sopenharmony_cifirst of all, thanks for submitting your patch! We really appreciate these 208cb93a386Sopenharmony_cisubmissions. After receiving an approval from a committer, you will be able to 209cb93a386Sopenharmony_ciclick the "Submit to CQ" button and submit your patch via the commit queue. 210cb93a386Sopenharmony_ci 211cb93a386Sopenharmony_ciIn special instances, a Skia committer may assist you in landing the change by 212cb93a386Sopenharmony_ciuploading a new codereview containing your patch (perhaps with some small 213cb93a386Sopenharmony_ciadjustments at their discretion). If so, you can mark your change as 214cb93a386Sopenharmony_ci"Abandoned", and update it with a link to the new codereview. 215cb93a386Sopenharmony_ci 216cb93a386Sopenharmony_ci### Skia committers 217cb93a386Sopenharmony_ci 218cb93a386Sopenharmony_ci- tips on how to apply an externally provided patch are [here](../patch) 219cb93a386Sopenharmony_ci- when landing externally contributed patches, please note the original 220cb93a386Sopenharmony_ci contributor's identity (and provide a link to the original codereview) in the 221cb93a386Sopenharmony_ci commit message 222cb93a386Sopenharmony_ci 223cb93a386Sopenharmony_ci `git-cl` will squash all your commits into a single one with the description 224cb93a386Sopenharmony_ci you used when you uploaded your change. 225cb93a386Sopenharmony_ci 226cb93a386Sopenharmony_ci ``` 227cb93a386Sopenharmony_ci git cl land 228cb93a386Sopenharmony_ci ``` 229cb93a386Sopenharmony_ci 230cb93a386Sopenharmony_ci or 231cb93a386Sopenharmony_ci 232cb93a386Sopenharmony_ci ``` 233cb93a386Sopenharmony_ci git cl land -c 'Contributor Name <email@example.com>' 234cb93a386Sopenharmony_ci ``` 235