Bug 1365255 - DevTools: bring over documentation for contributing and coding standards into the tree. r=nchevobbe

MozReview-Commit-ID: IRtCRTMmPHu

--HG--
extra : rebase_source : df1a6325a746a6408159b9f8223d82748ca27c13
This commit is contained in:
sole 2017-05-26 13:02:02 +01:00
Родитель 2d0337cf6f
Коммит 8c22801540
14 изменённых файлов: 505 добавлений и 2 удалений

Просмотреть файл

@ -6,6 +6,14 @@
* [Architecture overview](getting-started/architecture-overview.md)
* [Set up to build DevTools](getting-started/build.md)
* [Development profiles](getting-started/development-profiles.md)
* [Contributing](./contributing.md)
* [Coding standards](./contributing/coding-standards.md)
* [JavaScript](./contributing/javascript.md)
* [ESLint](./contributing/eslint.md)
* [CSS](./contributing/css.md)
* [Creating and sending patches](./contributing/making-prs.md)
* [Code reviews](./contributing/code-reviews.md)
* [Filing good bugs](./contributing/filing-good-bugs.md)
* [Bugs and issue trackers](bugs-issues.md)
* [Files and directories](files/README.md)
* [Adding New Files](files/adding-files.md)

Просмотреть файл

@ -0,0 +1,16 @@
# Contributing
<!--TODO: make this less code focused and take ideas from debugger.html's contributing.md file-->
Thank you for taking the time to contribute!
Many people become contributors to implement a feature they miss, or want to fix something that annoys them. If you want to help, but don't know where to start, you could [look at our list of things to do](./bugs-issues.md). Alternatively, you might want to [report an issue or request a feature](./filing-good-bugs.md).
If it's your first time contributing, you should also read the documentation on [coding standards](./contributing/coding-standards.md). This might help you with questions such as what style of code to use, how to name new files (if you have to add any), tools we use to run automated checks, etc.
Once you think your work is ready to be shared, [learn how to make and submit a patch](./contributing/making-prs.md).
A member of the team will look at your contribution, and [perform a code review](./contributing/code-reviews.md).
* If all goes well, your patch will be merged into the project. Congratulations!
* If there are changes to be made, the reviewer will let you know, and you can send the updated patch again.

Просмотреть файл

@ -0,0 +1,59 @@
# Code reviews
This checklist is primarily aimed at reviewers, as it lists important points to check while reviewing a patch.
It can also be useful for patch authors: if the changes comply with these guidelines, then it's more likely the review will be approved.
## Bug status and patch file
<!--TODO: update when we move to github-->
* Bug status is assigned, and assignee is correctly set.
* Commit title and message follow [the conventions](https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#Commit_Message_Conventions).
* Commit message says [what is being changed and why](http://mozilla-version-control-tools.readthedocs.org/en/latest/mozreview/commits.html#write-detailed-commit-messages).
* Patch applies locally to current sources with no merge required.
* Check that every new file introduced by the patch has the proper Mozilla license header: https://www.mozilla.org/en-US/MPL/headers/
## Manual testing
* Verify:
* if it's a new feature, the patch implements it.
* if it's a fix, the patch fixes the bug it addresses.
* Report any problems you find in the global review comment.
* Decide if any of those problems should block landing the change, or if they can be filed as follow-up bugs instead, to be fixed later.
## Automated testing
* Run new/modified tests, [with and without e10s](../tests/writing-tests.md#electrolysis).
* Watch out for tests that pass but log exceptions or end before protocol requests are handled.
* Watch out for slow/long tests: suggest many small tests rather than single long tests.
* Watch out for new tests written as integration tests instead of as unit tests: unit tests should be the preferred option, when possible.
## Code review
* Code changes:
* Review only what was changed by the contributor.
* Code formatting follows [our ESLint rules](eslint.md) and [coding standards](./coding-standards.md).
* Code is properly commented, JSDoc is updated, new "public" methods all have JSDoc, see the [comment guidelines](./javascript.md#comments).
* If Promise code was added/modified, the right promise syntax is used and rejections are handled. See [asynchronous code](./javascript.md#asynchronous-code).
* If a CSS file is added/modified, it follows [the CSS guidelines](./css.md).
* If a React or Redux module is added/modified, it follows the [React/Redux guidelines](./javascript.md#react--redux).
* If DevTools server code that should run in a worker is added/modified then it shouldn't use Services
* Test changes:
* The feature or bug is [tested by new tests, or a modification of existing tests](../tests/writing-tests.md).
* [Test logging](../tests/writing-tests.md#logs-and-comments) is sufficient to help investigating test failures/timeouts.
* [Test is e10s compliant](../tests/writing-tests.md#e10s-electrolysis) (no CPOWs, no content, etc…).
* Tests are [clean and maintainable](../tests/writing-tests.md#writing-clean-maintainable-test-code).
* A try push has started (or even better, is green already)<!--TODO review and update with mentions to Travis, Circle or whatever it is we use when we move to GitHub-->.
* User facing changes:
* If a new piece of UI or new user interaction is added/modified, then UX is `ui-review?` on the bug.<!--TODO this needs updating with the new process-->
* If a user facing string has been added, it is localized and follows [the localization guidelines](../files/adding-files.md#localization-l10n).
* If a user-facing string has changed meaning, [the key has been updated](https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Changing_existing_strings).
* If a new image is added, it is a SVG image or there is a reason for not using a SVG.
* If a SVG is added/modified, it follows [the SVG guidelines](../frontend/svgs.md).
* If a documented feature has been modified, the keyword `dev-doc-needed` is present on the bug.
## Finalize the review
<!--TODO update with the GitHub workflow when we're there-->
* R+: the code should land as soon as possible.
* R+ with comments: there are some comments, but they are minor enough, or don't require a new review once addressed, trust the author.
* R cancel / R- / F+: there is something wrong with the code, and a new review is required.

Просмотреть файл

@ -0,0 +1,10 @@
# Coding standards
Our code base is quite large, and a lot of different people contribute to it all the time. Therefore, it's important to share standards to keep the code consistent and written in a predictable style. This also helps avoid common mistakes.
We have pages defining standards, best practices and tips for various languages used in our tools:
* [JavaScript](./javascript.md)
* [CSS](./css.md)
* [SVG](../frontend/svgs.md)

Просмотреть файл

@ -0,0 +1,137 @@
# CSS
This page is for information about CSS used by DevTools. Wondering about the Dev Edition theme? See this page for more information about the [Developer Edition theme](https://wiki.mozilla.org/DevTools/Developer_Edition_Theme).
## Basics
The CSS code is in `devtools/client/themes`.
Here are some basic tips that can optimize reviews if you are changing CSS:
* Avoid `!important` but if you have to use it, make sure it's obvious why you're using it (maybe with a comment).
* Avoid magic numbers, prefer automatic sizing.
* Avoid platforms specific styles, put everything in the `shared` directory.
* Avoid preprocessor variables, use CSS variables instead.
* Avoid setting styles in JavaScript. It's generally better to set a class and then specify the styles in CSS
* `classList` is generally better than `className`. There's less chance of over-writing an existing class.
### Boilerplate
Make sure each file starts with the standard copyright header (see [License Boilerplate](https://www.mozilla.org/MPL/headers/)).
### Testing
CSS changes should generally be similar across platforms since they used a shared implementation, but there can still be differences worth checking out. Check major changes on Windows, OS X and Ubuntu.
## Formatting
We use 2-spaces indentation for the CSS.
In general the formatting looks like this:
```css
selector,
alternate-selector {
property: value;
other-property: other-value;
}
```
<!--TODO: add examples for long shorthand properties, and multi-valued properties (background, font-family, ...)-->
Also:
* Omit units on 0 values.
* Example: Use `margin: 0;`, not `margin: 0px;`.
* Add a space after each comma, **except** within color functions.
* Example: `linear-gradient(to bottom, black 1px, rgba(255,255,255,0.2) 1px)`.
* Always add a space before ` !important`.
* Assume `="true"` in attribute selectors.
* Example: Use `option[checked]`, not `option[checked="true"]`.
* Use longhand versions of properties so it's clear what you're changing.
* Example: Use `border-color: red`, not `border: red;`.
Naming standards for class names:
* `lower-case-with-dashes` is the most common.
* But `camelCase` is also used sometimes. Try to follow the style of existing or related code.
## Light and Dark theme support
DevTools supports 3 different themes: the dark theme, the light theme and the firebug theme. In order to support them, there are 3 class names available (`theme-dark`, `theme-light` and `theme-firebug`).
* Use [pre-defined CSS variables](https://developer.mozilla.org/en-US/docs/Tools/DevToolsColors) instead of hardcoding colors when possible.
* If you need to support themes and the pre-defined variables don't fit, define a variable with your custom colors at the beginning of the CSS file. This avoids selector duplication in the code.
Example:
```css
.theme-light {
--some-variable-name: <color-for-light-theme>;
}
.theme-dark {
--some-variable-name: <color-for-dark-theme>;
}
.theme-firebug {
--some-variable-name: <color-for-dark-theme>;
}
#myElement {
background-color: var(--some-variable-name);
}
```
## HDPI support
It's recommended to use SVG since it keeps the CSS clean when supporting multiple resolutions. However, if only 1x and 2x PNG assets are available, you can use this `@media` query to target higher density displays (HDPI): `@media (min-resolution: 1.1dppx)`. <!--TODO an example would be good here-->
## Performance
* Read [Writing Efficient CSS](https://developer.mozilla.org/en-US/docs/Web/Guide/CSS/Writing_efficient_CSS).
* Use an iframe where possible so your rules are scoped to the smallest possible set of nodes.<!--TODO: is this still true? and also refine exactly when it is appropriate to use an iframe. Examples might help-->
* If your CSS is used in `browser.xul`, you need to take special care with performance:
* Descendent selectors should be avoided.
* If possible, find ways to use **only** id selectors, class selectors and selector groups.
## Localization
### Text Direction
* For margins, padding and borders, use `inline-start`/`inline-end` rather than `left`/`right`.
* Example: Use `margin-inline-start: 3px;` not `margin-left: 3px`.
* For RTL-aware positioning (left/right), use `offset-inline-start/end`.
* When there is no special RTL-aware property (eg. `float: left|right`) available, use the pseudo `:-moz-locale-dir(ltr|rtl)` (for XUL files) or `:dir(ltr|rtl)` (for HTML files).
* Remember that while a tab content's scrollbar still shows on the right in RTL, an overflow scrollbar will show on the left.
* Write `padding: 0 3px 4px;` instead of `padding: 0 3px 4px 3px;`. This makes it more obvious that the padding is symmetrical (so RTL won't be an issue).
### RTL support for html modules
By default, new HTML modules support only left-to-right (LTR) and do not reuse the current direction of the browser.
To enable right-to-left (RTL) support in a module, set the `[dir]` attribute on the document element of the module:
* Example: `<html xmlns="http://www.w3.org/1999/xhtml" dir="">`.
The appropriate value for the `dir` attribute will then be set when the toolbox loads this module.
### Testing
The recommended workflow to test RTL on DevTools is to use the [Force RTL extension](https://addons.mozilla.org/en-US/firefox/addon/force-rtl/). After changing the direction using Force RTL, you should restart DevTools to make sure all modules apply the new direction. A future version of Force RTL will be able to update dynamically all DevTools documents.<!--TODO: update when the fate of this addon/webextension is known-->
Going to `about:config` and setting `intl.uidirection.en` to rtl is not recommended, and will always require to re-open DevTools to have any impact.
## Toggles
Sometimes you have a style that you want to turn on and off. For example a tree twisty (a expand-collapse arrow), a tab background, etc.
The Mozilla way is to perform the toggle using an attribute rather than a class:
```css
.tree-node {
background-image: url(right-arrow.svg);
}
.tree-node[open] {
background-image: url(down-arrow.svg);
}
```
## Tips
* Use `:empty` to match a node that doesn't have children.
* Usually, if `margin` or `padding` has 4 values, something is wrong. If the left and right values are asymmetrical, you're supposed to use `-start` and `-end`. If the values are symmetrical, use only 3 values (see localization section).

Двоичные данные
devtools/docs/contributing/eslint-atom-settings.png Normal file

Двоичный файл не отображается.

После

Ширина:  |  Высота:  |  Размер: 110 KiB

Двоичные данные
devtools/docs/contributing/eslint-atom.png Normal file

Двоичный файл не отображается.

После

Ширина:  |  Высота:  |  Размер: 786 KiB

Двоичные данные
devtools/docs/contributing/eslint-sublimetext3.png Normal file

Двоичный файл не отображается.

После

Ширина:  |  Высота:  |  Размер: 324 KiB

Двоичные данные
devtools/docs/contributing/eslint-vscode.png Normal file

Двоичный файл не отображается.

После

Ширина:  |  Высота:  |  Размер: 316 KiB

Просмотреть файл

@ -0,0 +1,158 @@
# Using ESLint in DevTools
<!--TODO paths, executables and everything here should be reviewed when we go to GitHub-->
The main rule set is in `devtools/.eslintrc`. It is meant to be used with ESLint 3.5.0.
Note that the file `.eslintignore` at the root of the repository contains a list of paths to ignore. This is because a lot of the code isn't ESLint compliant yet. We're in the process of making code free of ESLint warnings and errors, but this takes time. In the meantime, make sure the file or folder you are running ESLint on isn't ignored.
## Installing
From the root of the project type:
`./mach eslint --setup`
ESLint, `eslint-plugin-html`, `eslint-plugin-mozilla` and `eslint-plugin-react` will be automatically downloaded and installed.
## Running ESLint
### From the command line
The preferred way of running ESLint from the command line is by using `mach` like this:
```bash
./mach eslint path/to/directory
```
This ensures that ESLint runs with the same configuration that our CI environment (see the next section).
### In continuous integration
Relying only on people to run ESLint isn't enough to guarantee new warnings or errors aren't introduced in the code. Therefore, ESLint also runs automatically in our Continuous Integration environment.
This means that every time a commit is pushed to one of the repositories, a job runs ESLint on the whole code.
If you are pushing a patch to the [`try` repository](https://wiki.mozilla.org/ReleaseEngineering/TryServer) to run the tests, then you can also tell it to run the ESLint job and therefore verify that you did not introduce new errors.
If you build on all platforms, then the ESLint job will run by default, but if you selected a few platforms only in your [trysyntax](https://wiki.mozilla.org/Build:TryChooser), then you need to also add `eslint-gecko` as a target platform for ESLint to run.
### Running ESLint in SublimeText
SublimeText is a popular code editor and it supports ESLint via a couple of plugins. Here are some pointers to get started:
* make sure you have [SublimeText 3](http://www.sublimetext.com/3), the linter plugin doesn't work with ST2,
* install [SublimeLinter 3](http://www.sublimelinter.com/en/latest/installation.html), this is a framework for linters that supports, among others, ESLint. Installing SublimeLinter via [Package Control](https://packagecontrol.io/installation) is the easiest way)
* with SublimeLinter installed, you can now [install the specific ESLint plugin](https://github.com/roadhump/SublimeLinter-eslint#linter-installation). The installation instructions provide details about how to install node, npm, eslint which are required).
* make sure to configure SublimeLinter with the `--no-ignore` option so that errors are also shown for source files that are ignored. To do this, open the SublimeLinter user configuration at: Preferences / Package Settings / SublimeLinter / Settings - User, and add `"args": "--no-ignore"` to the eslint linter object.
You will also need to point SublimeLinter at the local eslint installation by setting the path to whatever `./mach eslint --setup` gives you when you run it (include a trailing slash but remove the eslint binary filename) e.g.
NOTE: Your local eslint binary is at /some-project-path/tools/lint/eslint/node_modules/.bin/eslint
```
"paths": {
"linux": [],
"osx": [
"/some-project-path/tools/lint/eslint/node_modules/.bin"
],
"windows": [
"C:\\some-project-path\\tools\\lint\\eslint\\node_modules\\.bin"
]
},
```
Once done, open the mozilla project in SublimeText and open any JS file in the `/devtools` directory. You can then trigger the linter via the contextual menu (right click on the file itself) or with a keyboard shortcut (ctrl+option+L on Mac).
You should see errors and warnings in the gutter as shown in the screenshot below. You can also see all errors listed with ctrl+option+A, and individual errors by clicking in the gutter marker.
![ESLint in SublimeText 3](./eslint-sublimetext3.png)
### Running ESLint in Emacs
* First, install the flycheck package (flymake doesn't support ESLint yet). You can get flycheck from the [marmalade](https://marmalade-repo.org/) or [melpa-stable](http://stable.melpa.org/#/) repositories.
* Tell flycheck to disable jslint, and enable flycheck in your javascript mode. Some care is needed to find the eslint installed in the source tree. This snippet assumes the built-in javascript mode, but with minor changes (just the name of the hook) should work fine with js2-mode as well:
```lisp
(defun my-js-mode-hacks ()
(setq-local mode-name "JS")
;; Set this locally so that the head.js rule continues to work
;; properly. In particular for a mochitest we want to preserve the
;; "browser_" prefix.
(when (buffer-file-name)
(let ((base (file-name-nondirectory (buffer-file-name))))
(when (string-match "^\\([a-z]+_\\)" base)
(setq-local flycheck-temp-prefix (match-string 1 base))))
(let ((base-dir (locate-dominating-file (buffer-file-name)
".eslintignore")))
(when base-dir
(let ((eslint (expand-file-name
"tools/lint/eslint/node_modules/.bin/eslint" base-dir)))
(when (file-exists-p eslint)
(setq-local flycheck-javascript-eslint-executable eslint))))))
(flycheck-mode 1))
(require 'flycheck)
(setq-default flycheck-disabled-checkers
(append flycheck-disabled-checkers
'(javascript-jshint)))
(add-hook 'js-mode-hook #'my-js-mode-hacks)
```
* flycheck puts its bindings on `C-c !` by default, so use `C-c ! C-h` to see what is available. There are key bindings to list all the errors and to move through the errors, among other things.
* To make sure flycheck is finding eslint, open a .js file and run `M-x flycheck-verify-setup`. It should show the path to your eslint installation.
### Running ESLint in Atom
From the root of the project type:
`./mach eslint --setup`
Install the [linter-eslint](https://atom.io/packages/linter-eslint) package v.8.00 or above. Then go to the package settings and enable the following options:
![linter-eslint settings in Atom](eslint-atom-settings.png)
Once done, you should see errors and warnings as shown in the screenshot below.
![ESLint in Atom](eslint-atom.png)
### Running ESLint in ViM
If you don't use Syntastic yet, the instructions here should get you started: https://wiki.mozilla.org/WebExtensions/Hacking#Vim
Alternatively, if you do use Syntastic already, add this to your `.vimrc` to get ESLint working where the path contains `mozilla-central` (adjust the path to reflect the one in your computer):
```vim
autocmd FileType javascript,html
\ if stridx(expand("%:p"), "/mozilla-central/") != -1 |
\ let b:syntastic_checkers = ['eslint'] |
\ let b:syntastic_eslint_exec = '/path/to/mozilla-central/tools/lint/eslint/node_modules/.bin/eslint' |
\ let b:syntastic_html_eslint_args = ['--plugin', 'html'] |
\ endif
```
You probably need to close and reopen ViM for the changes to take effect. Then, open any file and try to edit it to cause an error, then save it. If all goes well, you will get some distinctive arrows pointing to the error. Hovering with the mouse will produce a sort of tooltip with more information about the error.
### Running ESLint in Visual Studio Code
From the root of the project type:
`./mach eslint --setup`
Install the [dbaeumer.vscode-eslint](https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint) package. Then go to the package settings and set the following option:
`"eslint.nodePath": "tools/lint/eslint/node_modules/.bin"`
Once done, you should see errors and warnings as shown in the screenshot below:
![ESLint in VS Code](eslint-vscode.png)
### Fixing ESLint Errors
This should help you write eslint-clean code:
* When moving or refactoring a piece of code, consider this as an opportunity to remove all ESlint errors from this piece of code. In fact, it may even be a good opportunity to remove all ESLint errors from the entire file.
* When doing ESLint-only changes, please do them in a separate patch from the actual functionality changes or bug fix. This helps make the review easier, and isolate the actual changes when looking at the source history.
* When cleaning an entire file or folder from ESLint errors, do not forget to remove the corresponding entry from the `.eslintignore` file.
* When writing new code, from scratch, please make it ESLint compliant from the start. This is a lot easier than having to revisit it later.
* ESLint also runs on `<script>` tags in HTML files, so if you create new HTML test files for mochitests for example, make sure that JavaScript code in those files is free of ESLint errors.
* Depending on how a dependency is loaded into a file, the symbols this dependency exports might not be considered as defined by ESLint. For instance, using `Cu.import("some.jsm")` doesn't explicitly say which symbols are now available in the scope of the file, and so using those symbols will be consider by ESLint as using undefined variables. When this happens, please avoid using the `/* globals ... */` ESLint comment (which tells it that these variables are defined). Instead, please use `/* import-globals-from relative/path/to/file.js */`. This way, you won't have a list of variables to maintain manually, the globals are going to be imported dynamically instead.
* In test files (xpcshell and mochitest), all globals from the corresponding `head.js` file are imported automatically, so you don't need to define them using a `/* globals ... */` comment or a `/* import-globals-from head.js */` comment.

Просмотреть файл

@ -0,0 +1,11 @@
# Filing good bugs
Getting started working on a bug can be hard, specially if you lack context.
This guide is meant to provide a list of steps to provide the necessary information to resolve a bug.
* Use a descriptive title. Avoid jargon and abbreviations where possible, they make it hard for other people to find existing bugs, and to understand them.
* Explain the problem in depth and provide the steps to reproduce. Be as specific as possible, and include things such as operating system and version if reporting a bug.
* If you can, list files and lines of code that may need to be modified. Ideally provide a patch for getting started.
* If applicable, provide a test case or document that can be used to test the bug is solved. For example, if the bug title was "HTML inspector fails when inspecting a page with one million of nodes", you would provide an HTML document with one million of nodes, and we could use it to test the implementation, and make sure you're looking at the same thing we're looking at. You could use services like jsfiddle, codepen or jsbin to share your test cases. Other people use GitHub, or their own web server.
* If it's a bug that new contributors can work on, add the keyword `good-first-bug`.

Просмотреть файл

@ -0,0 +1,89 @@
# JavaScript coding standards
Probably the best piece of advice is **to be consistent with the rest of the code in the file**.
We use [ESLint](http://eslint.org/) to analyse JavaScript files automatically, either from within a code editor or from the command line. Here's [our guide to install and configure it](./eslint.md).
For quick reference, here are some of the main code style rules:
* file references to browser globals such as `window` and `document` need `/* eslint-env browser */` at the top of the file,
* lines should be 90 characters maximum,
* indent with 2 spaces (no tabs!),
* `camelCasePlease`,
* don't open braces on the next line,
* don't name function expressions: `let o = { doSomething: function doSomething() {} };`,
* use a space before opening paren for anonymous functions, but don't use one for named functions:
* anonymous functions: `function () {}`
* named functions: `function foo() {}`
* anonymous generators: `function* () {}`
* named generators: `function* foo() {}`
* aim for short functions, 24 lines max (ESLint has a rule that checks for function complexity too),
* `aArguments aAre the aDevil` (don't use them please),
* `"use strict";` globally per module,
* `semicolons; // use them`,
* no comma-first,
* consider using Task.jsm (`Task.async`, `Task.spawn`) for nice-looking asynchronous code instead of formatting endless `.then` chains,<!--TODO: shouldn't we advise async/await now?-->
* use ES6 syntax:
* `function setBreakpoint({url, line, column}) { ... }`,
* `(...args) => { }` rest args are awesome, no need for `arguments`,
* `for..of` loops,
* don't use non-standard SpiderMonkey-only syntax:
* no `for each` loops,
* no `function () implicitReturnVal`,
* getters / setters require { },
* only import specific, explicitly-declared symbols into your namespace:
* `const { foo, bar } = require("foo/bar");`,
* `const { foo, bar } = Cu.import("...", {});`,
* use Maps, Sets, WeakMaps when possible,
* use [template strings](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals) whenever possible to avoid concatenation, allow multi-line strings, and interpolation.
## Comments
Commenting code is important, but bad comments can hurt too, so it's important to have a few rules in mind when commenting:
* If the code can be rewritten to be made more readable, then that should be preferred over writing an explanation as a comment.
* Instead of writing a comment to explain the meaning of a poorly chosen variable name, then rename that variable.
* Avoid long separator comments like `// ****************** another section below **********`. They are often a sign that you should split a file in multiple files.
* Line comments go above the code they are commenting, not at the end of the line.
* Sentences in comments start with a capital letter and end with a period.
* Watch out for typos.
* Obsolete copy/pasted code hurts, make sure you update comments inside copy/pasted code blocks.
* A global comment at the very top of a file explaining what the file is about and the major types/classes/functions it contains is a good idea for quickly browsing code.
* If you are forced to employ some kind of hack in your code, and there's no way around it, then add a comment that explains the hack and why it is needed. The reviewer is going to ask for one anyway.
* Bullet points in comments should use stars aligned with the first comment to format each point
```javascript
// headline comment
// * bullet point 1
// * bullet point 2
```
## Asynchronous code
A lot of code in DevTools is asynchronous, because a lot of it relies on connecting to the DevTools debugger server and getting information from there in an asynchronous fashion.
It's easy to make mistakes with asynchronous code, so here are a few guidelines that should help:
* Prefer [promises](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise) over callbacks.
* Use the `new Promise(() => {})` syntax.
* Don't forget to catch rejections by defining a rejection handler: `promise.then(() => console.log("resolved"), () => console.log("rejected"));` or `promise.catch(() => console.log("rejected"));`.
* Make use of [`Tasks` and generator functions](https://developer.mozilla.org/en-US/docs/Mozilla/JavaScript_code_modules/Task.jsm) to make asynchronous code look synchronous. <!--TODO async/await as in the TODO above?-->
## React & Redux
There are React-specific code style rules in the .eslintrc file.
### Components
* Default to creating components as [stateless function components](https://facebook.github.io/react/docs/reusable-components.html#stateless-functions).
* If you need local state or lifecycle methods, use `React.createClass` instead of functions.
* Use React.DOM to create native elements. Assign it to a variable named `dom`, and use it like `dom.div({}, dom.span({}))`. You may also destructure specific elements directly: `const { div, ul } = React.DOM`.
### PropTypes
* Use [PropTypes](https://facebook.github.io/react/docs/reusable-components.html#prop-validation) to define the expected properties of your component. Each directly accessed property (or child of a property) should have a corresponding PropType.
* Use `isRequired` for any required properties.
* Place the propTypes definition at the top of the component. If using a stateless function component, place it above the declaration of the function.
* Where the children property is used, consider [validating the children](http://www.mattzabriskie.com/blog/react-validating-children).

Просмотреть файл

@ -0,0 +1,15 @@
# Creating and sending patches <!--TODO: (in the future: Making Pull Requests)-->
<!-- TODO: this will need to be updated in the future when we move to GitHub -->
Once you have a patch file, add it as an attachment to the Bugzilla ticket you are working on and add the `feedback?` or `review?` flag depending on if you just want feedback and confirmation you're doing the right thing or if you think the patch is ready to land respectively. Read more about [how to submit a patch and the Bugzilla review cycle here](https://developer.mozilla.org/en-US/docs/Developer_Guide/How_to_Submit_a_Patch).
You can also take a look at the [Code Review Checklist](./code-reviews.md) as it contains a list of checks that your reviewer is likely to go over when reviewing your code.
## Posting patches as gists
* Install gist-cli: `npm install -g gist-cli`
* In your bash profile add:
* git: `alias gist-patch="git diff | gist -o -t patch"`
* hg: `alias gist-patch="hg diff | gist -o -t patch"`
* Then go to a clean repo, modify the files, and run the command `gist-patch`

Просмотреть файл

@ -113,11 +113,11 @@ It's important to know whether or not the `shared.js` in your test directory alr
If you're planning to work on a lot of new tests, it might be worth the time actually importing `shared-head.js` in your `head.js` if it isn't here already.
## E10S (Electrolysis)
## Electrolysis
E10S is the codename for Firefox multi-process, and what that means for us is that the process in which the test runs isn't the same as the one in which the test content page runs.
You can learn more about E10S [from this blog post](https://timtaubert.de/blog/2011/08/firefox-electrolysis-101/) and [the Electrolysis wiki page](https://wiki.mozilla.org/Electrolysis).
You can learn more about E10S [from this blog post](https://timtaubert.de/blog/2011/08/firefox-electrolysis-101/), [the Electrolysis wiki page](https://wiki.mozilla.org/Electrolysis) and the page on [tests and E10s](https://wiki.mozilla.org/Electrolysis/e10s_test_tips).
One of the direct consequences of E10S on tests is that you cannot retrieve and manipulate objects from the content page as you'd do without E10S.