Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1709)

Unified Diff: site/dev/contrib/submit.md

Issue 844433004: Add Contributing to Skia section of docs (Closed) Base URL: https://skia.googlesource.com/skia.git@master
Patch Set: fix code block on flatten page Created 5 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « site/dev/contrib/style.md ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: site/dev/contrib/submit.md
diff --git a/site/dev/contrib/submit.md b/site/dev/contrib/submit.md
new file mode 100644
index 0000000000000000000000000000000000000000..fcbcf2077d48db5711ff3cd1d704ca07d311e072
--- /dev/null
+++ b/site/dev/contrib/submit.md
@@ -0,0 +1,248 @@
+How to submit a patch
+=====================
+
+
+Making changes
+--------------
+
+First create a branch for your changes:
+
+~~~~
+$ git checkout --track origin/master -b my_feature master
+~~~~
+
+After making your changes, create a commit
+
+~~~~
+$ git add [file1] [file2] ...
+$ git commit
+~~~~
+
+If your branch gets out of date, you will need to update it:
+
+~~~~
+$ git pull --rebase
+$ gclient sync
+~~~~
+
+
+Adding a unit test
+------------------
+
+If you are willing to change Skia codebase, it's nice to add a test at the same
+time. Skia has a simple unittest framework so you can add a case to it.
+
+Test code is located under the 'tests' directory. Assuming we are adding
+tests/FooTest.cpp, The test code will look like:
+
+<!--?prettify?-->
+~~~~
+/*
+ * Copyright ........
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "Test.h"
+
+DEF_TEST(TestFoo, reporter) {
+ int x = 2 * 3;
+ if (x != 6) {
+ ERRORF(reporter, "x should be 6, but is %d", x);
+ return;
+ }
+ REPORTER_ASSERT(reporter, 1 + 1 == 2);
+}
+~~~~
+
+And we need to add this new file to gyp/tests.gyp. Note that file names are
+sorted alphabetically.
+
+<!--?prettify?-->
+~~~~
+'sources': [
+ '../tests/AAClipTest.cpp'
+ '../tests/FooTest.cpp',
+ '../tests/XfermodeTest.cpp',
+],
+~~~~
+
+Unit tests are best, but if your change touches rendering and you can't think of
+an automated way to verify the results, consider writing a GM test or a new page
+of SampleApp. Also, if your change is the GPU code, you may not be able to write
+it as part of the standard unit test suite, but there are GPU-specific testing
+paths you can extend.
+
+
+Submitting a patch
+------------------
+
+For your code to be accepted into the codebase, you must complete the
+[Individual Contributor License
+Agreement](http://code.google.com/legal/individual-cla-v1.0.html). You can do
+this online, and it only takes a minute. If you are contributing on behalf of a
+corporation, you must fill out the [Corporate Contributor License Agreement](http://code.google.com/legal/corporate-cla-v1.0.html)
+and send it to us as described on that page. Add your (or your organization's)
+name and contact info to the AUTHORS file as a part of your CL.
+
+Now that you've made a change and written a test for it, it's ready for the code
+review! Submit a patch and getting it reviewed is fairly easy with depot tools.
+
+Use git-cl, which comes with [depot tools](http://sites.google.com/a/chromium.org/dev/developers/how-tos/install-depot-tools).
+For help, run git-cl help.
+
+### Configuring git-cl
+
+Before using any git-cl commands you will need to configure it to point at the
+correct code review server. This is accomplished with the following command:
+
+~~~~
+git cl config https://skia.googlesource.com/skia/+/master/codereview.settings
+~~~~
+
+### Find a reviewer
+
+Ideally, the reviewer is someone who is familiar with the area of code you are
+touching. If you have doubts, look at the git blame for the file to see who else
+has been editing it.
+
+### Uploading changes for review
+
+Skia uses Chromium's code review [site](http://codereview.chromium.org) and the
+Rietveld open source code review tool.
+Use git cl to upload your change:
+~~~~
+$ git cl upload
+~~~~
+
+You may have to enter a Google Account username and password to authenticate
+yourself to codereview.chromium.org. A free gmail account will do fine, or any
+other type of Google account. It does not have to match the email address you
+configured using git config --global user.email above, but it can.
+
+The command output should include a URL, similar to
+(https://codereview.chromium.org/111893004/), indicating where your changelist
+can be reviewed.
+
+### Request review
+
+Go to the supplied URL or go to the code review page and click **Issues created
+by me**. Select the change you want to submit for review and click **Edit
+Issue**. Enter at least one reviewer's email address and click **Update Issue**.
+Now click on **Publish+Mail Comments**, add any optional notes, and send your
+change off for review. Unless you publish your change, no one will know to look
+at it.
+
+_Note_: If you don't see editing commands on the review page, click **Log In**
+in the upper right. _Hint_: You can add -r reviewer@example.com --send-mail to
+send the email directly when uploading a change in both gcl and git-cl.
+
+
+The review process
+------------------
+
+If you submit a giant patch, or do a bunch of work without discussing it with
+the relevant people, you may have a hard time convincing anyone to review it!
+
+Please follow the guidelines on how to conduct a code review detailed here:
+https://code.google.com/p/rietveld/wiki/CodeReviewHelp
+
+Code reviews are an important part of the engineering process. The reviewer will
+almost always have suggestions or style fixes for you, and it's important not to
+take such suggestions personally or as a commentary on your abilities or ideas.
+This is a process where we work together to make sure that the highest quality
+code gets submitted!
+
+You will likely get email back from the reviewer with comments. Fix these and
+update the patch set in the issue by uploading again. The upload will explain
+that it is updating the current CL and ask you for a message explaining the
+change. Be sure to respond to all comments before you request review of an
+update.
+
+If you need to update code the code on an already uploaded CL, simply edit the
+code, commit it again locally, and then run git cl upload again e.g.
+
+~~~~
+echo "" > GOATS
+git add GOATS
+git commit -m 'add newline fix to GOATS'
+git cl upload
+~~~~
+
+Once you're ready for another review, use **Publish+Mail Comments** again to
+send another notification (it is helpful to tell the review what you did with
+respect to each of their comments). When the reviewer is happy with your patch,
+they will say "LGTM" ("Looks Good To Me").
+
+_Note_: As you work through the review process, both you and your reviewers
+should converse using the code review interface, and send notes using
+**Publish+Mail Comments**.
+
+Once your commit has gone in, you should delete the branch containing your change:
+
+~~~~
+$ git checkout master
+$ git branch -D my_feature
+~~~~
+
+
+Final Testing
+-------------
+
+Skia's principal downstream user is Chromium, and any change to Skia rendering
+output can break Chromium. If your change alters rendering in any way, you are
+expected to test for and alleviate this. (You may be able to find a Skia team
+member to help you, but the onus remains on each individual contributor to avoid
+breaking Chrome.
+
+### Evaluating Impact on Chromium
+
+Keep in mind that Skia is rolled daily into Blink and Chromium. Run local tests
+and watch canary bots for results to ensure no impact. If you are submitting
+changes that will impact layout tests, follow the guides below and/or work with
+your friendly Skia-Blink engineer to evaluate, rebaseline, and land your
+changes.
+
+Resources:
+
+[How to land Skia changes that change Blink layout test results](../chrome/layouttest)
+
+If you're changing the Skia API, you may need to make an associated change in Chromium.
+If you do, please follow these instructions: [Landing Skia changes which require Chrome changes](../chrome/changes)
+
+
+Check in your changes
+---------------------
+
+### Non-Skia-committers
+
+If you already have committer rights, you can follow the directions below to
+commit your change directly to Skia's repository.
+
+If you don't have committer rights in https://skia.googlesource.com/skia.git ...
+first of all, thanks for submitting your patch! We really appreciate these
+submissions. Unfortunately, we don't yet have a way for Skia committers to mark
+a patch as "approved" and thus allow non-committers to commit them. So instead,
+please ask a Skia committer to land your patch for you or land using the commit
+queue.
+
+As part of this process, the Skia committer may create a new codereview
+containing your patch (perhaps with some small adjustments at her discretion).
+If so, you can mark your codereview as "Closed", and update it with a link to
+the new codereview.
+
+### Skia committers:
+ * tips on how to apply the externally provided patch are [here](./patch)
+ * when landing externally contributed patches, please note the original
+ contributor's identity (and provide a link to the original codereview) in the commit message
+
+git-cl will squash all your commits into a single one with the description you used when you uploaded your change.
+
+~~~~
+git cl land
+~~~~
+or
+~~~~
+git cl land -c 'Contributor Name <email@example.com>'
+~~~~
« no previous file with comments | « site/dev/contrib/style.md ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698