DevTools/ReviewTips: Difference between revisions

From MozillaWiki
Jump to navigation Jump to search
Line 64: Line 64:
** A patch that cleanly applies to fx-team
** 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)
** A patch with a message in the format 'Bug XXXXXX - Message' (pro-tip: cut and paste from the Bug page)
** The bugs 'depends' field should correctly list any bugs that should be committed before this one
** 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)


Other sources of hints and tips:
Other sources of hints and tips:

Revision as of 16:03, 3 November 2011

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
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)

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
  • 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.