DevTools/ReviewTips
From MozillaWiki
< DevTools
Contents
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:
Headers
- Make sure each file starts with the standard copyright header (see License Boilerplate)
CSS
- See:
- Reviewers: dao
JavaScript
- See:
- 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.