Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update contributing docs #1574

Merged
merged 5 commits into from
Oct 9, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .eslintrc
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
{
"rules":{
"no-console":0
"no-console": 0
},
"env": {
"node": true,
Expand Down
144 changes: 131 additions & 13 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,20 +1,138 @@
Thanks for wanting to contribute to Mozilla's Add-ons Linter! You rock! 😊
Hi! Thanks for wanting to contribute to Mozilla's Add-ons Linter! You rock! 😊
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tenor


## Submitting a Pull Request
This linter is used to help develop and publish [Add-ons](https://developer.mozilla.org/en-US/Add-ons/) for Firefox. You're an add-on developer, we would really value your contributions–no one knows add-on development and publishing better than an add-on developer!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to remove the /en-US/ directory and let MDN redirect to the user's preferred locale.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh good point, damn copy & paste. Thanks!


If you're submitting a pull request, please reference the issue it closes
inside your pull request. Keep your commits atomic; you should have one
commit per issue solved.
Here are links to all the sections in this document:

Add tests for your code and make sure all existing tests pass. If the tests
fail or you don't maintain 100% test coverage we won't be able to accept your
pull request.
<!-- If you change any of the headings in this document, remember to update the table of contents. -->
<!-- To update the TOC, run the command `npm run gen-contributing-toc` from your root directory and you will auto generate a new TOC. -->

### Tests
<!-- START doctoc generated TOC please keep comment here to allow auto update -->
<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->

Our tests include `eslint` style checks; these keep our code consistent.
You can review the rules in the [`.eslintrc`][eslint] file.

Please run the tests locally with `npm test` before you commit.
- [Picking an issue](#picking-an-issue)
- [Installation](#installation)
- [Testing the linter](#testing-the-linter)
- [Run all tests](#run-all-tests)
- [Run a single test](#run-a-single-test)
- [Debug a test](#debug-a-test)
- [Build addons-linter](#build-addons-linter)
- [Creating a pull request](#creating-a-pull-request)
- [Writing commit messages](#writing-commit-messages)
- [Tips](#tips)

[eslint]: https://github.com/mozilla/addons-linter/blob/master/.eslintrc
<!-- END doctoc generated TOC please keep comment here to allow auto update -->


# Picking an issue

For first-time contributors or those who want to start with a small task: [check out our list of good first bugs](https://github.com/mozilla/addons-linter/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+bug%22). These issues have an assigned mentor to help you out and are great issues for learning about the linter and our development process.

If you're already familiar with the project or would like take on something a little more challenging, please take a look at the [contrib: welcome](https://github.com/mozilla/addons-linter/issues?q=is%3Aissue+is%3Aopen+label%3A"contrib%3A+welcome) issues.

If you'd like to work on a bug, please comment on it to let the maintainers know.
If someone else has already commented and taken up that bug, please refrain from working on it and submitting
a PR without asking the maintainers as it leads to unnecessary duplication of effort.


# Installation

To get started on a patch, first install `addons-linter` from [source](README.md#development).

# Testing the linter

To run tests and check for JavaScript syntax issues as you change the code, run:

npm test

## Run all tests

To run the entire suite of tests once and exit, type:

npm run test-once

This is the same as the `npm test` command but it won't re-run automatically as
you edit files.

## Run a single test

Instead of running the entire suite, you can run a single test by invoking
the `jest` executable directly with the `-t` option to filter by test
description. For example, if the test you'd like to run is defined in
`tests/test.linter.js` and is described as
"should collect an error when not an xpi/zip" then you could run it like this:

./node_modules/.bin/jest -r tests/test.linter.js -f "not an xpi/zip"

## Debug a test

You can enter the [Node debugger](https://nodejs.org/api/debugger.html) by
directly invoking the `npm run debug` command. For example,
if the test you want to debug is defined in `tests/test.linter.js` then you
could enter the debugger like this:

node --inspect --inspect-brk ./node_modules/.bin/jest tests/test.linter.js -t 'flag potentially minified'

You could also put the `debugger` statement somewhere in the code to set a
breakpoint.

# Build addons-linter

Type `npm run build` to build a new version of the libraries used by the
`./bin/addons-linter` command. When successful, you will see newly built files in
the `./dist/` directory.

# Creating a pull request

When you create a
[pull request](https://help.github.com/articles/creating-a-pull-request/)
for a new fix or feature, be sure to mention the issue
number for what you're working on.
The best way to do it is to mention the issue like
this at the top of your description:

Fixes #123

The issue number in this case is "123."
The word *Fixes* is magical; GitHub will automatically close the issue when your
pull request is merged.

# Writing commit messages

Good commit messages serve at least three important purposes:

* They speed up the reviewing process.
* They help us write good release notes.
* They help future maintainers understand your change and the reasons behind it.

Structure your commit message like this:

From: [[http://git-scm.com/book/ch5-2.html]]

> ```
> Short (50 chars or less) summary of changes
>
> More detailed explanatory text, if necessary. Wrap it to about 72
> characters or so. In some contexts, the first line is treated as the
> subject of an email and the rest of the text as the body. The blank
> line separating the summary from the body is critical (unless you omit
> the body entirely); tools like rebase can get confused if you run the
> two together.
>
> Further paragraphs come after blank lines.
>
> - Bullet points are okay, too
>
> - Typically a hyphen or asterisk is used for the bullet, preceded by a
> single space, with blank lines in between, but conventions vary here
> ```

* Write the summary line and description of what you have done in the imperative mode, that is as if you were commanding someone. Start the line with "Fix", "Add", "Change" instead of "Fixed", "Added", "Changed".
* Always leave the second line blank.
* Be as descriptive as possible in the description. It helps reasoning about the intention of commits and gives more context about why changes happened.

Tips
----

* If it seems difficult to summarize what your commit does, it may be because it includes several logical changes or bug fixes, and are better split up into several commits using `git add -p`.
32 changes: 3 additions & 29 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ bin/addons-linter my-addon.zip

### Required node version

addons-linter requires node.js v4 or greater. Using nvm is probably the
addons-linter requires node.js v6 or greater. Using nvm is probably the
easiest way to manage multiple node versions side by side. See
[nvm on github](https://github.com/creationix/nvm) for more details.

Expand All @@ -135,10 +135,6 @@ Install dependencies with [yarn](https://yarnpkg.com/lang/en/docs/install/):
yarn install
```





Dependencies are automatically kept up-to-date using [greenkeeper](http://greenkeeper.io/).

### yarn scripts and grunt tasks
Expand All @@ -160,28 +156,6 @@ Most of these scripts will also run with npm by substuting `yarn` with `npm`.
| yarn start | Builds the lib and watches for changes |
| yarn [run] build | Builds the lib (used by Travis) |

If you install `grunt-cli` globally then you can run other tasks.

```
npm install -g grunt-cli
```

From the grunt docs:

> The job of the Grunt CLI is simple: run the version of Grunt which has
been installed next to a Gruntfile. This allows multiple versions of
Grunt to be installed on the same machine simultaneously.

#### Grunt tasks

| Script | Description |
|------------------------|--------------------------------------------------|
| grunt test | Runs the tests |
| grunt build | Builds the lib |
| grunt start | Builds the lib and watches for changes |
| grunt eslint | Lints the files with eslint (Run in grunt test) |


### Building and watching for changes

You can run `yarn start` to build the library and then rebuild on file changes.
Expand Down Expand Up @@ -239,10 +213,10 @@ available](http://chaijs.com/api/assert/)
We use [bunyan](https://github.com/trentm/node-bunyan) for logging:

* By default logging is off (level is set to 'fatal') .
* Logging in tests can be enabled using an env var e.g: `LOG_LEVEL=debug grunt test`
* Logging in tests can be enabled using an env var e.g: `LOG_LEVEL=debug jest test`
* Logging on the cli can be enabled with `--log-level [level]`.
* Bunyan by default logs JSON. If you want the json to be pretty printed
pipe anything that logs into `bunyan` e.g. `LOG_LEVEL=debug grunt test
pipe anything that logs into `bunyan` e.g. `LOG_LEVEL=debug jest test
| node_modules/bunyan/bin/bunyan`


Expand Down
6 changes: 4 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
"test-ci": "npm run test-coverage-once && cat ./coverage/lcov.info | coveralls",
"lint": "npm run eslint",
"publish-rules": "grunt copy build-rules-html publish-rules",
"publish-coverage": "grunt coveralls"
"publish-coverage": "grunt coveralls",
"gen-contributing-toc": "doctoc CONTRIBUTING.md"
},
"repository": {
"type": "git",
Expand Down Expand Up @@ -74,9 +75,10 @@
"babel-register": "6.26.0",
"chalk": "2.1.0",
"cheerio": "1.0.0-rc.2",
"common-tags": "1.4.0",
"columnify": "1.5.4",
"common-tags": "1.4.0",
"crx-parser": "0.1.2",
"doctoc": "1.3.0",
"dispensary": "0.10.19",
"es6-promisify": "5.0.0",
"eslint": "4.8.0",
Expand Down