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