Changes

Jump to: navigation, search

Webmaker/Code

1,217 bytes added, 19:57, 11 October 2013
4. Learn the Webmaker Development Workflow
# Click '''Submit''' and wait for your reviewer to carry out the review.
===3. Get your code Reviewed=== Code review is a [http://vocamus.net/dave/?p=1569 critical component of the Mozilla project]. Code review increases code quality, distributes knowledge of changes across a team, allows for mentorship of new developers, etc. All changes in Webmaker need review, from a single line change to a 5,000 line feature addition. There are no exceptions, and no one is "good enough" to skip having their code reviewed. In order to get your code reviewed, you first need to find an appropriate reviewer. There are a few strategies. The first is to ask in the bug, on irc, or the dev list. The second is to look at <code>git blame {filename}</code> information, which will show you who touched the code you're working in last, and often who did the review (e.g., the commit message or bug that changed it will have that info). Your reviewer will use a mixture of inline Github code comments and comments in the Bugzilla bug. Generally speaking, issues relating to the code itself will go in Github, and general comments in Bugzilla. As your code is being reviewed, you will likely be asked to make fixes. This will include fixing errors, adding comments, adhering to prevailing styles, etc. Here is how you do it:
# Go back to your bug fix branch: <code>git checkout bug12345</code>
# Make your changes, commit, and push to your remote repo. The existing pull request will get updated.
# If your reviewer has given you an '''r-''', you may need to ask for review again. You can do this by clicking '''Details''' beside your attachment in Bugzilla, then resetting the '''review''' flag to '''?''' from '''-'''. Don't update your branch to the latest code in Mozilla's master branch yet--we'll do that when it's ready to land and we rebase (see below).
Don't be discouraged when this process takes multiple tries. All developers make mistakes, or lack knowledge of the entire project sufficient to catch issues outside the particular code in question. It's common for a bug to take many updates before the code is ready to get checked in (aka., ''landed''). When it's time to land your code, your reviewer will likely ask you to '''rebase'''. It is highly recommended that you ask for help your first time, unless you have experience doing this, as it will alter your branch's commit history. We prefer rebase over merge so that our repositories have clean, single-change master branches, which makes reverting and bisecting much easier. A rebase generally goes like this:
At this point you can ask your reviewer to land your changes, either via irc, or in a comment in the bug.
TODO:* ===4. Verify your code reviewon Staging===* pull requests, inline comments* flags/whiteboard things to know about (e.g., l10n string changes)TODO
* tagging
* pushing to staging/production
<center>http://farm8.staticflickr.com/7340/9337213776_d77d88eb89.jpg</center>
Confirm
656
edits

Navigation menu