blob: bebb94606a0fab46b894577e35bb71b45bbd6fe6 [file] [log] [blame] [view]
Joe Gregorio02f72022021-03-27 10:12:45 -04001---
Joe Gregorioe296c562021-04-05 11:39:40 -04002title: 'How to submit a patch'
3linkTitle: 'How to submit a patch'
Joe Gregorio02f72022021-03-27 10:12:45 -04004---
5
Joe Gregorioe296c562021-04-05 11:39:40 -04006## Configure git
Joe Gregorio02f72022021-03-27 10:12:45 -04007
8<!--?prettify lang=sh?-->
9
10 git config --global user.name "Your Name"
11 git config --global user.email you@example.com
12
Joe Gregorioe296c562021-04-05 11:39:40 -040013## Making changes
Joe Gregorio02f72022021-03-27 10:12:45 -040014
15First create a branch for your changes:
16
17<!--?prettify lang=sh?-->
18
19 git config branch.autosetuprebase always
Ravi Mistrye5be65e2021-05-20 16:01:33 -040020 git checkout -b my_feature origin/main
Joe Gregorio02f72022021-03-27 10:12:45 -040021
22After making your changes, create a commit
23
24<!--?prettify lang=sh?-->
25
26 git add [file1] [file2] ...
27 git commit
28
29If your branch gets out of date, you will need to update it:
30
31<!--?prettify lang=sh?-->
32
33 git pull
34 python2 tools/git-sync-deps
35
Joe Gregorioe296c562021-04-05 11:39:40 -040036## Adding a unit test
Joe Gregorio02f72022021-03-27 10:12:45 -040037
38If you are willing to change Skia codebase, it's nice to add a test at the same
39time. Skia has a simple unittest framework so you can add a case to it.
40
41Test code is located under the 'tests' directory.
42
Joe Gregorioe296c562021-04-05 11:39:40 -040043See [Writing Unit and Rendering Tests](/docs/dev/testing/tests) for details.
Joe Gregorio02f72022021-03-27 10:12:45 -040044
45Unit tests are best, but if your change touches rendering and you can't think of
Joe Gregorioe296c562021-04-05 11:39:40 -040046an automated way to verify the results, consider writing a GM test. Also, if
47your change is in the GPU code, you may not be able to write it as part of the
48standard unit test suite, but there are GPU-specific testing paths you can
49extend.
Joe Gregorio02f72022021-03-27 10:12:45 -040050
Joe Gregorioe296c562021-04-05 11:39:40 -040051## Submitting a patch
Joe Gregorio02f72022021-03-27 10:12:45 -040052
53For your code to be accepted into the codebase, you must complete the
Joe Gregorioe296c562021-04-05 11:39:40 -040054[Individual Contributor License Agreement](http://code.google.com/legal/individual-cla-v1.0.html).
55You can do this online, and it only takes a minute. If you are contributing on
56behalf of a corporation, you must fill out the
57[Corporate Contributor License Agreement](http://code.google.com/legal/corporate-cla-v1.0.html)
Joe Gregorio02f72022021-03-27 10:12:45 -040058and send it to us as described on that page. Add your (or your organization's)
59name and contact info to the AUTHORS file as a part of your CL.
60
61Now that you've made a change and written a test for it, it's ready for the code
62review! Submit a patch and getting it reviewed is fairly easy with depot tools.
63
Joe Gregorioe296c562021-04-05 11:39:40 -040064Use `git-cl`, which comes with
65[depot tools](http://sites.google.com/a/chromium.org/dev/developers/how-tos/install-depot-tools).
66For help, run `git cl help`. Note that in order for `git cl` to work correctly,
67it needs to run on a clone of <https://skia.googlesource.com/skia>. Using clones
68of mirrors, including Google's mirror on GitHub, might lead to issues with
69`git cl` usage.
Joe Gregorio02f72022021-03-27 10:12:45 -040070
71### Find a reviewer
72
73Ideally, the reviewer is someone who is familiar with the area of code you are
74touching. Look at the git blame for the file to see who else has been editing
Joe Gregorioe296c562021-04-05 11:39:40 -040075it. If unsuccessful, another option is to click on the 'Suggested Reviewers'
76button to add one of the listed Skia contacts. They should be able to add
77appropriate reviewers for your change. The button is located here:
78<img src="/docs/dev/contrib/SuggestedReviewers.png" style="display: inline-block; max-width: 75%" />
Joe Gregorio02f72022021-03-27 10:12:45 -040079
80### Uploading changes for review
81
Joe Gregorioe296c562021-04-05 11:39:40 -040082Skia uses the Gerrit code review tool. Skia's instance is
83[skia-review](http://skia-review.googlesource.com). Use `git cl` to upload your
84change:
Joe Gregorio02f72022021-03-27 10:12:45 -040085
86<!--?prettify lang=sh?-->
87
88 git cl upload
89
90You may have to enter a Google Account username and password to authenticate
Joe Gregorioe296c562021-04-05 11:39:40 -040091yourself to Gerrit. A free gmail account will do fine, or any other type of
92Google account. It does not have to match the email address you configured using
93`git config --global user.email` above, but it can.
Joe Gregorio02f72022021-03-27 10:12:45 -040094
95The command output should include a URL, similar to
96(https://skia-review.googlesource.com/c/4559/), indicating where your changelist
97can be reviewed.
98
99### Submit try jobs
100
101Skia's trybots allow testing and verification of changes before they land in the
102repo. You need to have permission to trigger try jobs; if you need permission,
Joe Gregorioe296c562021-04-05 11:39:40 -0400103ask a committer. After uploading your CL to
104[Gerrit](https://skia-review.googlesource.com/), you may trigger a try job for
105any job listed in tasks.json, either via the Gerrit UI, using `git cl try`, eg.
Joe Gregorio02f72022021-03-27 10:12:45 -0400106
107 git cl try -B skia.primary -b Some-Tryjob-Name
108
Joe Gregorioe296c562021-04-05 11:39:40 -0400109or using bin/try, a small wrapper for `git cl try` which helps to choose try
110jobs. From a Skia checkout:
Joe Gregorio02f72022021-03-27 10:12:45 -0400111
112 bin/try --list
113
114You can also search using regular expressions:
115
116 bin/try "Test.*GTX660.*Release"
117
Joe Gregorioe296c562021-04-05 11:39:40 -0400118For more information about testing, see
119[testing infrastructure](/docs/dev/testing/automated_testing).
Joe Gregorio02f72022021-03-27 10:12:45 -0400120
121### Request review
122
123Go to the supplied URL or go to the code review page and select the **Your**
124dropdown and click on **Changes**. Select the change you want to submit for
Joe Gregorioe296c562021-04-05 11:39:40 -0400125review and click **Reply**. Enter at least one reviewer's email address. Now add
126any optional notes, and send your change off for review by clicking on **Send**.
127Unless you send your change to reviewers, no one will know to look at it.
Joe Gregorio02f72022021-03-27 10:12:45 -0400128
129_Note_: If you don't see editing commands on the review page, click **Sign in**
130in the upper right. _Hint_: You can add -r reviewer@example.com --send-mail to
131send the email directly when uploading a change using `git-cl`.
132
Joe Gregorioe296c562021-04-05 11:39:40 -0400133## The review process
Joe Gregorio02f72022021-03-27 10:12:45 -0400134
135If you submit a giant patch, or do a bunch of work without discussing it with
136the relevant people, you may have a hard time convincing anyone to review it!
137
138Code reviews are an important part of the engineering process. The reviewer will
139almost always have suggestions or style fixes for you, and it's important not to
140take such suggestions personally or as a commentary on your abilities or ideas.
141This is a process where we work together to make sure that the highest quality
142code gets submitted!
143
144You will likely get email back from the reviewer with comments. Fix these and
145update the patch set in the issue by uploading again. The upload will explain
146that it is updating the current CL and ask you for a message explaining the
147change. Be sure to respond to all comments before you request review of an
148update.
149
150If you need to update code the code on an already uploaded CL, simply edit the
151code, commit it again locally, and then run git cl upload again e.g.
152
153 echo "GOATS" > whitespace.txt
154 git add whitespace.txt
155 git commit -m 'add GOATS fix to whitespace.txt'
156 git cl upload
157
158Once you're ready for another review, use **Reply** again to send another
159notification (it is helpful to tell the reviewer what you did with respect to
160each of their comments). When the reviewer is happy with your patch, they will
161approve your change by setting the Code-Review label to "+1".
162
163_Note_: As you work through the review process, both you and your reviewers
164should converse using the code review interface, and send notes.
165
166Once your change has received an approval, you can click the "Submit to CQ"
167button on the codereview page and it will be committed on your behalf.
168
Joe Gregorioe296c562021-04-05 11:39:40 -0400169Once your commit has gone in, you should delete the branch containing your
170change:
Joe Gregorio02f72022021-03-27 10:12:45 -0400171
Ravi Mistrye5be65e2021-05-20 16:01:33 -0400172 git checkout -q origin/main
Joe Gregorio02f72022021-03-27 10:12:45 -0400173 git branch -D my_feature
174
Joe Gregorioe296c562021-04-05 11:39:40 -0400175## Final Testing
Joe Gregorio02f72022021-03-27 10:12:45 -0400176
177Skia's principal downstream user is Chromium, and any change to Skia rendering
178output can break Chromium. If your change alters rendering in any way, you are
179expected to test for and alleviate this. You may be able to find a Skia team
180member to help you, but the onus remains on each individual contributor to avoid
181breaking Chrome.
182
183### Evaluating Impact on Chromium
184
Joe Gregorioe296c562021-04-05 11:39:40 -0400185Keep in mind that Skia is rolled daily into Blink and Chromium. Run local tests
186and watch canary bots for results to ensure no impact. If you are submitting
Joe Gregorio02f72022021-03-27 10:12:45 -0400187changes that will impact layout tests, follow the guides below and/or work with
188your friendly Skia-Blink engineer to evaluate, rebaseline, and land your
189changes.
190
191Resources:
192
Joe Gregorioe296c562021-04-05 11:39:40 -0400193[How to land Skia changes that change Blink layout test results](/docs/dev/chrome/blink/)
Joe Gregorio02f72022021-03-27 10:12:45 -0400194
Joe Gregorioe296c562021-04-05 11:39:40 -0400195If you're changing the Skia API, you may need to make an associated change in
196Chromium. If you do, please follow these instructions:
197[Landing Skia changes which require Chrome changes](/docs/dev/chrome/changes/)
Joe Gregorio02f72022021-03-27 10:12:45 -0400198
Joe Gregorioe296c562021-04-05 11:39:40 -0400199## Check in your changes
Joe Gregorio02f72022021-03-27 10:12:45 -0400200
201### Non-Skia-committers
202
203If you already have committer rights, you can follow the directions below to
204commit your change directly to Skia's repository.
205
206If you don't have committer rights in https://skia.googlesource.com/skia.git ...
Joe Gregorioe296c562021-04-05 11:39:40 -0400207first of all, thanks for submitting your patch! We really appreciate these
208submissions. After receiving an approval from a committer, you will be able to
Joe Gregorio02f72022021-03-27 10:12:45 -0400209click the "Submit to CQ" button and submit your patch via the commit queue.
210
Joe Gregorioe296c562021-04-05 11:39:40 -0400211In special instances, a Skia committer may assist you in landing the change by
212uploading a new codereview containing your patch (perhaps with some small
213adjustments at their discretion). If so, you can mark your change as
Joe Gregorio02f72022021-03-27 10:12:45 -0400214"Abandoned", and update it with a link to the new codereview.
215
216### Skia committers
Joe Gregorio02f72022021-03-27 10:12:45 -0400217
Joe Gregorioe296c562021-04-05 11:39:40 -0400218- tips on how to apply an externally provided patch are [here](../patch)
219- when landing externally contributed patches, please note the original
220 contributor's identity (and provide a link to the original codereview) in the
221 commit message
Joe Gregorio02f72022021-03-27 10:12:45 -0400222
Joe Gregorioe296c562021-04-05 11:39:40 -0400223 `git-cl` will squash all your commits into a single one with the description
224 you used when you uploaded your change.
Joe Gregorio02f72022021-03-27 10:12:45 -0400225
Joe Gregorioe296c562021-04-05 11:39:40 -0400226 ```
227 git cl land
228 ```
Joe Gregorio02f72022021-03-27 10:12:45 -0400229
Joe Gregorioe296c562021-04-05 11:39:40 -0400230 or
Joe Gregorio02f72022021-03-27 10:12:45 -0400231
Joe Gregorioe296c562021-04-05 11:39:40 -0400232 ```
233 git cl land -c 'Contributor Name <email@example.com>'
234 ```