wiki:CommitterTips
Last modified 11 months ago Last modified on 06/04/13 01:30:49

Tips and Tools for Committers and Reviewers

You're reading this because you're a newly minted WebKit Committer or Reviewer. See the WebKit Committer and Reviewer Policy for more information.

Becoming a committer

  1. After you've fulfilled the requirements outlined in the WebKit Committer and Reviewer Policy, a reviewer will nominate you on webkit-reviewers@lists.webkit.org.
  2. 2 other reviewers must second your nomination.
  3. After 5 business days your nomination carries and someone from Apple (most often Brian Weinstein) will contact you with paperwork to sign and send back.
  4. After your paperwork has been received by Apple, someone will contact you (most often Brian Weinstein) with information about your login credentials.

Becoming a reviewer

  1. After you've fulfilled the requirements outlined in the WebKit Committer and Reviewer Policy, a reviewer will nominate you on webkit-reviewers@lists.webkit.org.
  2. 2 other reviewers must second your nomination
  3. After 5 business days your nomination carries and a reviewer will contact you to make sure you accept the nomination.
  4. Afterwards, a reviewer will post on the webkit blog to announce your acceptance and congratulate you!

Lists you should be on once you have commit/review access

Tools you should know to use

  • webkit-patch see --help for more information. Most useful are the land and upload subcommands.
  • svn-apply is how we apply patches from bugzilla. (webkit-patch apply and webkit-patch apply-from-bug use svn-apply under the covers.)
  • svn-apply has various helpful flags including --reviewer.

Walking you through your first commit

Below are some "manual" steps for doing your first commit from an SVN checkout. In some cases, alternative commands are listed.

WebKit maintains several tools that simplify the commit process in many scenarios. All of these tools are scripts in the Tools/Scripts directory. Nevertheless, understanding more manual forms of the process is still a good thing. Because the tools can't do everything, it helps to have the manual process to fall back on in case the tools can't do what you need.

  1. Create an account.

To commit code, you will need an account. To create an account, register your e-mail address at http://trac.webkit.org and choose a password. You may be given a webkit.org e-mail address to use for this account.

  1. Configure commit-log-editor.

When using "svn commit" to submit a patch, the commit message should be the ChangeLog message(s) for the patch. WebKit maintains a script called "commit-log-editor" that populates the commit message for you. To configure the editor that commit-log-editor opens, you can set the SVN_LOG_EDITOR environment variable to the editor of your choice (e.g. nano). See the commit-log-editor source code http://trac.webkit.org/browser/trunk/Tools/Scripts/commit-log-editor for alternative options and to see how commit-log-editor decides what editor to use.

  1. Set up Subversion.

Subversion has a handy feature called auto-props that will ensure that new binary files you add to the repository are given the correct MIME type. To set this up, find the following two lines in ~/.subversion/config and uncomment them:

enable-auto-props = yes

*.png = svn:mime-type=image/png

Note that this will work even if you use git-svn to commit changes to WebKit.

  1. Apply the patch to your local checkout.
Use the "svn-apply" script to apply the patch to your local checkout. Make sure the date and reviewer are correct in the ChangeLogs (e.g. by using the -rreviewer flag). Alternatively, if the patch has been submitted to http://bugs.webkit.org, you can use "webkit-patch apply-from-bug". This script will find the patch and set the reviewer for you.
  1. Update your local checkout, if necessary.

If time has elapsed since applying the patch to your local checkout, you may need to update your checkout with the latest repository changes. If an update causes a conflict in any ChangeLog files, you can use the resolve-ChangeLogs script to quickly resolve these conflicts and ensure that your ChangeLog changes are at the top.

Alternatively, you can use the "update-webkit" script, which is smart enough to run resolve-ChangeLogs automatically, if necessary.

  1. Commit the changes.

Use the following command to commit changes--

svn commit --editor-cmd "<path_to_commit-log-editor>"

Alternatively, you can set the SVN_EDITOR environment variable to "commit-log-editor" and ensure that the directory containing commit-log-editor is in your path environment variable.

A newer alternative is the following--

webkit-patch land

Note: The first time you commit, your SVN client may not pass your correct username (unless you use the --username option). In this case, simply hit enter at the first password prompt. This will cause a username prompt to appear and then a subsequent password prompt. For the username prompt, type in the username you registered in http://trac.webkit.org above (i.e. your account e-mail address). Your username will then be cached for future uses of the client.

  1. Register the commit in bugs.webkit.org.

If you are committing a patch in http://bugs.webkit.org, you should add a comment to the report with text like the following:

Manually committed r53667: http://trac.webkit.org/changeset/53667

Then change the status of the report to RESOLVED.

  1. Make sure you didn't break anything.

Monitor http://build.webkit.org/console to make sure your patch doesn't turn any of the buildbots red!

Unreviewed commits

Unreviewed commits should replace "Reviewed By..." by "Unreviewed" in the ChangeLog entry. An explanation for why it is not reviewed may follow.

For example:

Unreviewed. Fix broken build.
Unreviewed gardening.

etc.

Rolling out a commit

Here are some guidelines for rolling out (i.e. undoing) a commit.

  • Rolling out a commit should undo everything but the ChangeLog entries associated to a commit.
  • A commit that rolls out a prior commit should itself contain ChangeLog entries (e.g. prepared using prepare-Changelog).
  • The commands patch -R, svn-unapply, webkit-patch rollout, and git revert are all helpful for rolling out.

You can use webkitbot in the #webkit freenode irc channel for easier rollouts.

Guidelines for committers

  • Build fixes do not need review, but please mention in the changelog and in the original bug report what you fixed and which revision caused the fail.
    • examples for good comments: Buildfix after r100000 for !ENABLE(FOO) platforms. ; Typo fix after r100000. ; Revert accidentally committed lines after r100000.
    • examples for bad comments: Trivial GTK buildfix. ; Qt minimal buildfix. ; Revert accidentally committed lines.
  • Committers are the gate-keepers of the project. That's why you signed the agreement. Only commit patches which have been reviewed.
  • Getting your patch reviewed
  • Getting your bug looked at

Guidelines for reviewers

  • There are no "protected areas" of the code. No areas where only one reviewer has say. Reviewers should review anything they feel comfortable reviewing. If you are looking for experts in a certain area, trac and svn's annotate feature can help show you who has changed the relevant lines of code recently.
  • Reviewers should come to consensus before landing. If another reviewer objects, it's better to mark the patch r- and discuss than to land a patch without consensus.