• 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    python 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 or a new page
49of SampleApp. Also, if your change is the GPU code, you may not be able to write
50it as part of the standard unit test suite, but there are GPU-specific testing
51paths you can extend.
52
53Submitting a patch
54------------------
55
56For your code to be accepted into the codebase, you must complete the
57[Individual Contributor License
58Agreement](http://code.google.com/legal/individual-cla-v1.0.html). You can do
59this online, and it only takes a minute. If you are contributing on behalf of a
60corporation, you must fill out the [Corporate Contributor License
61Agreement](http://code.google.com/legal/corporate-cla-v1.0.html)
62and send it to us as described on that page. Add your (or your organization's)
63name and contact info to the AUTHORS file as a part of your CL.
64
65Now that you've made a change and written a test for it, it's ready for the code
66review! Submit a patch and getting it reviewed is fairly easy with depot tools.
67
68Use git-cl, which comes with [depot
69tools](http://sites.google.com/a/chromium.org/dev/developers/how-tos/install-depot-tools).
70For help, run git-cl help.
71
72### Find a reviewer
73
74Ideally, the reviewer is someone who is familiar with the area of code you are
75touching. If you have doubts, look at the git blame for the file to see who else
76has been editing it.
77
78### Uploading changes for review
79
80Skia uses the Gerrit code review tool. Skia's instance is [skia-review](http://skia-review.googlesource.com).
81Use git cl to upload your change:
82
83<!--?prettify lang=sh?-->
84
85    git cl upload
86
87You may have to enter a Google Account username and password to authenticate
88yourself to Gerrit. A free gmail account will do fine, or any
89other type of Google account.  It does not have to match the email address you
90configured using `git config --global user.email` above, but it can.
91
92The command output should include a URL, similar to
93(https://skia-review.googlesource.com/c/4559/), indicating where your changelist
94can be reviewed.
95
96### Request review
97
98Go to the supplied URL or go to the code review page and select the **Your**
99dropdown and click on **Changes**. Select the change you want to submit for
100review and click **Reply**. Enter at least one reviewer's email address. Now
101add any optional notes, and send your change off for review by clicking on
102**Send**. Unless you send your change to reviewers, no one will know to look
103at it.
104
105_Note_: If you don't see editing commands on the review page, click **Sign in**
106in the upper right. _Hint_: You can add -r reviewer@example.com --send-mail to
107send the email directly when uploading a change using git-cl.
108
109
110The review process
111------------------
112
113If you submit a giant patch, or do a bunch of work without discussing it with
114the relevant people, you may have a hard time convincing anyone to review it!
115
116Code reviews are an important part of the engineering process. The reviewer will
117almost always have suggestions or style fixes for you, and it's important not to
118take such suggestions personally or as a commentary on your abilities or ideas.
119This is a process where we work together to make sure that the highest quality
120code gets submitted!
121
122You will likely get email back from the reviewer with comments. Fix these and
123update the patch set in the issue by uploading again. The upload will explain
124that it is updating the current CL and ask you for a message explaining the
125change. Be sure to respond to all comments before you request review of an
126update.
127
128If you need to update code the code on an already uploaded CL, simply edit the
129code, commit it again locally, and then run git cl upload again e.g.
130
131    echo "GOATS" > whitespace.txt
132    git add whitespace.txt
133    git commit -m 'add GOATS fix to whitespace.txt'
134    git cl upload
135
136Once you're ready for another review, use **Reply** again to send another
137notification (it is helpful to tell the review what you did with respect to each
138of their comments). When the reviewer is happy with your patch, they will
139approve your change by setting the Code-Review label to "+1".
140
141_Note_: As you work through the review process, both you and your reviewers
142should converse using the code review interface, and send notes.
143
144Once your change has received an approval, you can click the "Submit to CQ"
145button on the codereview page and it will be committed on your behalf.
146
147Once your commit has gone in, you should delete the branch containing your change:
148
149    git checkout -q origin/master
150    git branch -D my_feature
151
152
153Final Testing
154-------------
155
156Skia's principal downstream user is Chromium, and any change to Skia rendering
157output can break Chromium. If your change alters rendering in any way, you are
158expected to test for and alleviate this. (You may be able to find a Skia team
159member to help you, but the onus remains on each individual contributor to avoid
160breaking Chrome.
161
162### Evaluating Impact on Chromium
163
164Keep in mind that Skia is rolled daily into Blink and Chromium.  Run local tests
165and watch canary bots for results to ensure no impact.  If you are submitting
166changes that will impact layout tests, follow the guides below and/or work with
167your friendly Skia-Blink engineer to evaluate, rebaseline, and land your
168changes.
169
170Resources:
171
172[How to land Skia changes that change Blink layout test results](../chrome/layouttest)
173
174If you're changing the Skia API, you may need to make an associated change in Chromium.
175If you do, please follow these instructions: [Landing Skia changes which require Chrome changes](../chrome/changes)
176
177
178Check in your changes
179---------------------
180
181### Non-Skia-committers
182
183If you already have committer rights, you can follow the directions below to
184commit your change directly to Skia's repository.
185
186If you don't have committer rights in https://skia.googlesource.com/skia.git ...
187first of all, thanks for submitting your patch!  We really appreciate these
188submissions.  After receiving an approval from a committer, you will be able to
189click the "Submit to CQ" button and submit your patch via the commit queue.
190
191In special instances, a Skia committer may assist you in landing the change
192by uploading a new codereview containing your patch (perhaps with some small
193adjustments at his/her discretion).  If so, you can mark your change as
194"Abandoned", and update it with a link to the new codereview.
195
196### Skia committers
197  *  tips on how to apply an externally provided patch are [here](./patch)
198  *  when landing externally contributed patches, please note the original
199     contributor's identity (and provide a link to the original codereview) in the commit message
200
201    git-cl will squash all your commits into a single one with the description you used when you uploaded your change.
202
203    ~~~~
204    git cl land
205    ~~~~
206
207    or
208
209    ~~~~
210    git cl land -c 'Contributor Name <email@example.com>'
211    ~~~~
212