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