From 6bf0a5cb5034a7e684dcc3500e841785237ce2dd Mon Sep 17 00:00:00 2001 From: Daniel Baumann Date: Sun, 7 Apr 2024 19:32:43 +0200 Subject: Adding upstream version 1:115.7.0. Signed-off-by: Daniel Baumann --- .../contributing/code-reviews-checklist.md | 58 +++ .../contributing/code-reviews-find-reviewer.md | 5 + .../contributor/contributing/code-reviews-setup.md | 27 ++ .../docs/contributor/contributing/code-reviews.md | 104 ++++ .../contributor/contributing/coding-standards.md | 9 + devtools/docs/contributor/contributing/css.md | 130 +++++ .../contributing/eslint-atom-settings.png | Bin 0 -> 51839 bytes .../docs/contributor/contributing/eslint-atom.png | Bin 0 -> 441312 bytes .../contributing/eslint-sublimetext3.png | Bin 0 -> 192344 bytes .../contributor/contributing/eslint-vscode.png | Bin 0 -> 233507 bytes devtools/docs/contributor/contributing/eslint.md | 157 ++++++ .../contributor/contributing/filing-good-bugs.md | 11 + .../docs/contributor/contributing/find-bugs.md | 6 + .../docs/contributor/contributing/fixing-bugs.md | 177 +++++++ .../docs/contributor/contributing/javascript.md | 87 ++++ .../docs/contributor/contributing/landing-code.md | 18 + .../docs/contributor/contributing/levelling-up.md | 23 + .../docs/contributor/contributing/making-prs.md | 82 ++++ .../docs/contributor/contributing/performance.md | 152 ++++++ .../performance/perfherder-results.png | Bin 0 -> 34127 bytes .../performance/profiler-buffer-size.png | Bin 0 -> 52565 bytes .../performance/profiler-chrome-url.png | Bin 0 -> 85613 bytes .../performance/profiler-custom-markers.png | Bin 0 -> 19934 bytes .../performance/profiler-filter-js.png | Bin 0 -> 4027 bytes .../performance/profiler-filter-require.png | Bin 0 -> 94321 bytes .../performance/profiler-main-thread.png | Bin 0 -> 8694 bytes .../performance/profiler-netmon-open-fixed.png | Bin 0 -> 123216 bytes .../performance/profiler-netmonitor-open.png | Bin 0 -> 137737 bytes .../performance/profiler-resource-url.png | Bin 0 -> 119168 bytes .../contributing/react-performance-tips.md | 527 +++++++++++++++++++++ 30 files changed, 1573 insertions(+) create mode 100644 devtools/docs/contributor/contributing/code-reviews-checklist.md create mode 100644 devtools/docs/contributor/contributing/code-reviews-find-reviewer.md create mode 100644 devtools/docs/contributor/contributing/code-reviews-setup.md create mode 100644 devtools/docs/contributor/contributing/code-reviews.md create mode 100644 devtools/docs/contributor/contributing/coding-standards.md create mode 100644 devtools/docs/contributor/contributing/css.md create mode 100644 devtools/docs/contributor/contributing/eslint-atom-settings.png create mode 100644 devtools/docs/contributor/contributing/eslint-atom.png create mode 100644 devtools/docs/contributor/contributing/eslint-sublimetext3.png create mode 100644 devtools/docs/contributor/contributing/eslint-vscode.png create mode 100644 devtools/docs/contributor/contributing/eslint.md create mode 100644 devtools/docs/contributor/contributing/filing-good-bugs.md create mode 100644 devtools/docs/contributor/contributing/find-bugs.md create mode 100644 devtools/docs/contributor/contributing/fixing-bugs.md create mode 100644 devtools/docs/contributor/contributing/javascript.md create mode 100644 devtools/docs/contributor/contributing/landing-code.md create mode 100644 devtools/docs/contributor/contributing/levelling-up.md create mode 100644 devtools/docs/contributor/contributing/making-prs.md create mode 100644 devtools/docs/contributor/contributing/performance.md create mode 100644 devtools/docs/contributor/contributing/performance/perfherder-results.png create mode 100644 devtools/docs/contributor/contributing/performance/profiler-buffer-size.png create mode 100644 devtools/docs/contributor/contributing/performance/profiler-chrome-url.png create mode 100644 devtools/docs/contributor/contributing/performance/profiler-custom-markers.png create mode 100644 devtools/docs/contributor/contributing/performance/profiler-filter-js.png create mode 100644 devtools/docs/contributor/contributing/performance/profiler-filter-require.png create mode 100644 devtools/docs/contributor/contributing/performance/profiler-main-thread.png create mode 100644 devtools/docs/contributor/contributing/performance/profiler-netmon-open-fixed.png create mode 100644 devtools/docs/contributor/contributing/performance/profiler-netmonitor-open.png create mode 100644 devtools/docs/contributor/contributing/performance/profiler-resource-url.png create mode 100644 devtools/docs/contributor/contributing/react-performance-tips.md (limited to 'devtools/docs/contributor/contributing') diff --git a/devtools/docs/contributor/contributing/code-reviews-checklist.md b/devtools/docs/contributor/contributing/code-reviews-checklist.md new file mode 100644 index 0000000000..566c152fa5 --- /dev/null +++ b/devtools/docs/contributor/contributing/code-reviews-checklist.md @@ -0,0 +1,58 @@ +# Code reviews checklist + +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 + +* Bug status is assigned, and assignee is correctly set. +* Commit title and message follow [the conventions](https://firefox-source-docs.mozilla.org/mobile/android/geckoview/contributor/contributing-to-mc.html). +* 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#electrolysis) (doesn't try to access web content from the parent process, 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). +* User facing changes: + * If any user-facing interfaces are added/modified, double-check the changes with the UX mockups or specs, if available. If there's any confusion, need-info the UX designer. + * 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://mozilla-l10n.github.io/documentation/localization/making_string_changes.html). + * 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 + +* 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. diff --git a/devtools/docs/contributor/contributing/code-reviews-find-reviewer.md b/devtools/docs/contributor/contributing/code-reviews-find-reviewer.md new file mode 100644 index 0000000000..67548b5c30 --- /dev/null +++ b/devtools/docs/contributor/contributing/code-reviews-find-reviewer.md @@ -0,0 +1,5 @@ +# Finding suitable reviewers + +There are several options to find a good reviewer for a patch. If the bug you are working on is mentored, assign the review to the mentor. Otherwise, assign it to the triage owner (visible in the "People" section of a Bug in Bugzilla). + +Finally, an easy option is to use the #devtools-reviewers group in Phabricator. diff --git a/devtools/docs/contributor/contributing/code-reviews-setup.md b/devtools/docs/contributor/contributing/code-reviews-setup.md new file mode 100644 index 0000000000..a55f114c1a --- /dev/null +++ b/devtools/docs/contributor/contributing/code-reviews-setup.md @@ -0,0 +1,27 @@ +# Set up for code reviews + +There are two things you need to do before you can get a code review, although you only need to do this once 😃 + +## Set up to get code reviews in Phabricator + +We use an online tool called Phabricator for code reviews. To create an account in Phabricator, you first need the Bugzilla account that you created earlier. If you don't have one, [create it now](../getting-started/bugzilla.md). + +--- + +⚠️ *IMPORTANT:* ⚠️️️ + +It's helpful to have the same user name in both Bugzilla and Phabricator, so that people always know how to find you. + +Bugzilla's `Real name` field can be edited after the fact, but you cannot change Phabricator's username once the account has been created. + +If you added an `:ircnickname` in your Bugzilla's `Real name`, Phabricator will use that to pre-fill the username field when you create the account. **Please double check you like the proposed username, and make any corrections before you register**. + +--- + +Once you understand the above, please [create a Phabricator account](https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html#creating-an-account). + + + +## Set up to send code for review + +In order to push your commit to Phabricator, you need to install [moz-phab](https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html#using-moz-phab). diff --git a/devtools/docs/contributor/contributing/code-reviews.md b/devtools/docs/contributor/contributing/code-reviews.md new file mode 100644 index 0000000000..9ddb03b5bb --- /dev/null +++ b/devtools/docs/contributor/contributing/code-reviews.md @@ -0,0 +1,104 @@ +# Code reviews + +A review is required before code is added to Mozilla's repository. In addition, contrary to what you might have seen in other open source projects, code is not pushed directly to the repository. Instead, once the code is approved by reviewers, they will _request_ the code to be _landed_ on the repository. + +All this can be done somehow manually, but we have infrastructure in place to help us with reviews and landing code. + +Learn about how to get started with getting your code reviewed and landed in our [setting up](./code-reviews-setup.md) guide. + +And read on to learn about why and how we do code reviews. + +## Why do we do code reviews? + +### Quality + +Doing code reviews helps with **correctness**. It gives us a chance to check that the code: + +- fixes the problem, +- doesn't have bugs, +- doesn't regress other things, +- covers edge cases. + +A review is also a chance for the reviewer to check that the right **test coverage** is here, that the change doesn't have **performance** problems, that it **simplifies hard to understand code** and that it comes with **code comments** and **adequate documentation**. + +### Learning and sharing knowledge + +While going through the process of doing a code review, **both the reviewer and the reviewee** will be learning something (a new part of the code, a new efficient way to write code, tests, localization, etc.). + +Making the code easy to understand by the reviewer helps everybody. + +Doing reviews also helps people working on DevTools feel more comfortable and learn about new parts of the codebase. + +It helps build consensus around ideas and practices. + +### Maintenance + +Doing reviews gives us an opportunity to check we are willing to maintain and support the new code that being introduced for a feature or bug fix. + +### Code consistency + +It is also a great opportunity for us to check that whatever new code is being written is consistent with what already exists in the code base. + +### Shared responsibility + +Approving a code review means that you are the second person who thinks this change is correct and a good idea. Doing this makes you responsible for the code change just as much as the author. + +It is the entire DevTools group who owns the code, not just the author. We write code as a group, not as individuals, because on the long-term it's the group that maintains it. + +Having a review discussion shares the ownership of the code, because authors and reviewers all put a little bit of themselves in it, and the code that results isn't just the result of one person's work + +## What should be avoided in code reviews? + +- Style nits that a linter should ideally handle. +- Requests that are outside of the scope of the bug. + +## What are some good practices for code reviews? + +### Use empathetic language. + +More reading: + +- [Mindful Communication in Code Reviews](http://amyciavolino.com/assets/MindfulCommunicationInCodeReviews.pdf) +- [How to Do Code Reviews Like a Human](https://mtlynch.io/human-code-reviews-1/) + +### Prefer smaller commits over large monolithic commits. + +It makes it easier for the reviewer to provide a quality review. It's also easier to get consensus on the change if that change is small. Finally, it's easier to understand the risk, and find regressions. + +### Be explicit + +Specifically, be explicit about required versus optional changes. + +Reviews are conversations, and the reviewee should feel comfortable on discussing and pushing back on changes before making them. + +### Be quick + +As a reviewer, strive to answer in a couple of days, or at least under a week. + +If that is not possible, please update the bug to let the requester know when you'll get to the review. Or forward it to someone else who has more time. + +### Ask for help + +Know when you're out of your comfort zone as a reviewer, and don't hesitate to ask for an extra review to someone else. + +It's fine, you can't know everything, and the more people participate, the more everybody learns, and the more bugs we avoid. + +## How do we communicate? + +First and foremost, like in any Mozilla-run platforms or events, please abide by [the Community Participation Guidelines](https://www.mozilla.org/en-US/about/governance/policies/participation/). + +Maintainers should **lead by example through their tone and language**. We want to build an inclusive and safe environment for working together. + +As a reviewer, **double-check your comments**. Just because you're a reviewer and think you have a better solution doesn't mean that's true. Assume **the code author has spent more time thinking about this part of the code than you have** (if you're the reviewer) and might actually be right, even if you originally thought something was wrong. It doesn't take long to look up the code and double-check. + +**Being inclusive** is highly important. We shouldn't make any assumptions about the person requesting a review, or about the person you're asking a review from, and always provide as much information as required, in a way that is as inclusive as possible. + +The bug will live forever online, and many people will read it long after the author and reviewers are done. + +Talking over video/IRC/Slack is a great way to get buy-in and share responsibility over a solution. It is also helpful to resolve disagreement and work through issues in a more relaxed environment. + +**Do not criticize** the reviewee, talk about the quality of the code on its own, and not directly how the reviewee wrote the code. + +Take the time to thank and point out good code changes. + +**Using "please" and “what do you think?”** goes a long way in making others feel like colleagues, and not subordinates. diff --git a/devtools/docs/contributor/contributing/coding-standards.md b/devtools/docs/contributor/contributing/coding-standards.md new file mode 100644 index 0000000000..43d6a496e9 --- /dev/null +++ b/devtools/docs/contributor/contributing/coding-standards.md @@ -0,0 +1,9 @@ +# 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) diff --git a/devtools/docs/contributor/contributing/css.md b/devtools/docs/contributor/contributing/css.md new file mode 100644 index 0000000000..ed54c2dfcf --- /dev/null +++ b/devtools/docs/contributor/contributing/css.md @@ -0,0 +1,130 @@ +# 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; +} +``` + +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 2 different themes: the dark theme and the light theme. In order to support them, there are 2 class names available (`theme-dark` and `theme-light`). + +* Use [pre-defined CSS variables](https://searchfox.org/mozilla-central/source/devtools/client/themes/variables.css) 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: ; +} +.theme-dark { + --some-variable-name: ; +} +#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)`. + +## Performance + +* Use an iframe where possible so your rules are scoped to the smallest possible set of nodes. +* If your CSS is used in `browser.xhtml`, 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 `inset-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: ``. + +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. + +## 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). diff --git a/devtools/docs/contributor/contributing/eslint-atom-settings.png b/devtools/docs/contributor/contributing/eslint-atom-settings.png new file mode 100644 index 0000000000..1108c206dc Binary files /dev/null and b/devtools/docs/contributor/contributing/eslint-atom-settings.png differ diff --git a/devtools/docs/contributor/contributing/eslint-atom.png b/devtools/docs/contributor/contributing/eslint-atom.png new file mode 100644 index 0000000000..52350a31ec Binary files /dev/null and b/devtools/docs/contributor/contributing/eslint-atom.png differ diff --git a/devtools/docs/contributor/contributing/eslint-sublimetext3.png b/devtools/docs/contributor/contributing/eslint-sublimetext3.png new file mode 100644 index 0000000000..8a76f06cda Binary files /dev/null and b/devtools/docs/contributor/contributing/eslint-sublimetext3.png differ diff --git a/devtools/docs/contributor/contributing/eslint-vscode.png b/devtools/docs/contributor/contributing/eslint-vscode.png new file mode 100644 index 0000000000..41a58fe9d4 Binary files /dev/null and b/devtools/docs/contributor/contributing/eslint-vscode.png differ diff --git a/devtools/docs/contributor/contributing/eslint.md b/devtools/docs/contributor/contributing/eslint.md new file mode 100644 index 0000000000..88da959255 --- /dev/null +++ b/devtools/docs/contributor/contributing/eslint.md @@ -0,0 +1,157 @@ +# Using ESLint in DevTools + + +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 `