DevTools/ReviewTips

General

During Development

  • Ask for feedback early. You can ask anyone for feedback, but asking a reviewer might speed up the process of getting review later
  • Don't leave it until the last moment to write tests
  • cedricv has some awesome code coverage tooling to make your tests better

Mercurial

[ui]
username = Your Name <your@email.address>

[defaults]
diff = -U 8 -p
qdiff = -U 8
qnew = -U

[diff]
git = 1
showfunc = 1
nodates = 1

[extensions]
hgext.mq =
  • Other nice things for your ~/.hgrc:
[defaults]
commit = --verbose
blame = --user --date --quiet --number --line-number
qseries = --summary

[alias]
qlog = log --stat -r qtip:qbase

[extensions]
fetch =
hgext.graphlog =
color =
rebase =

# http://mercurial.selenic.com/wiki/ColorExtension
[color]
qseries.applied = green
qseries.unapplied = yellow
qseries.missing = red

Getting Review

  • Ask for review when you believe that it's ready to land, which means:
    • Tests (which pass)
    • A patch that cleanly applies to fx-team
    • A patch with a message in the format 'Bug XXXXXX - Message' (pro-tip: cut and paste from the Bug page)

Other sources of hints and tips:

Headers

  • Make sure each file starts with the standard copyright header (see License Boilerplate)
    • For work funded by Mozilla, the 'Initial Developer of the Original Code' should be 'The Mozilla Foundation'.
    • Remember the year
    • The first letter of contributors names should be under the 'n' of 'contributors'
    • The original author of the code should have the string ' (original author)' after his/her email address

CSS

JavaScript

Localization (l10n) / DTDs and Properties

  • Moving files is annoying because localizers are language experts but not HG experts and they work off a different repo with a similar structure, so they have to mirror your file movements
  • Reviewers: dcamp, msucan, robcee for normal work, but ask Axel Hecht (aka :pike, aka l10n) for file moves or anything significant
  • All our strings files should begin with this stanza (See bug 689685)
# The correct localization of this file might be to keep it in
# English, or another language commonly spoken among web developers.
# You want to make that choice consistent across the developer tools.
# A good criteria is the language in which you'd find the best
# documentation on web development on the web.