DevTools/ReviewTips

General

• See also User:GavinSharp/Code_Review

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

• See https://developer.mozilla.org/en/Mercurial for lots of useful stuff
• Mercurial branches are a bit heavyweight for our development needs. MQ helps
• Your ~/.hgrc should include:

[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
annotate = --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
# Add these if your terminal has a black background, or they will appear black on black
status.ignored = white bold
qseries.unapplied = cyan bold
branches.closed = white bold

[paths]
try = ssh://hg.mozilla.org/try

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)
  • The bug's 'depends' field should correctly list any bugs that should be committed before this one (some reviewers test the patch, and you'll need it to land anyway)

• File followup bugs immediately after they are requested by reviewers, it's too easy to forget to do that.

Other sources of hints and tips:

• Brian Bondy's Cheatsheet
• FX-Team's 'Get Involved' Page

Headers

• Make sure each file starts with the standard copyright header (see License Boilerplate)

CSS

• See:
  • DevTools/CSSTips
• Reviewers: dao

JavaScript

• See:
  • JavaScript Style Guide
  • Mozilla Coding Style Guide
• Reviewers: dcamp, joe_walker, mikeratcliffe, paul

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
• Add concise localization notes to your strings to explain where they are found and their purpose.
• Rename the entity/property if you change the string.
• Always use the ellipsis character "…" instead of "...".
• Always use XHTML for pages, never HTML. DTDs do not load in HTML documents, so they cannot be localized.
• When you need access keys, read the accesskey FAQ and policies.
• Reviewers: dcamp, ddahl, 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.