Skip to content
This repository has been archived by the owner on Jan 20, 2019. It is now read-only.

Commit

Permalink
Merge pull request #121 from sangm/master
Browse files Browse the repository at this point in the history
[RFC] Replace ember-cli-eslint with the standard eslint
  • Loading branch information
rwjblue authored Nov 1, 2018
2 parents 1bef7c6 + e9eb29f commit 72baee5
Showing 1 changed file with 75 additions and 0 deletions.
75 changes: 75 additions & 0 deletions active/0000-remove-ember-cli-eslint.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
- Start Date: 2018-08-13
- RFC PR: (leave this empty)

# Summary

Remove https://github.com/ember-cli/ember-cli-eslint from projects generated by
`ember-cli`.

[ember-cli-eslint](https://github.com/ember-cli/ember-cli-eslint) is an addon
designed to show lint errors during test runs. Tooling around `eslint` has
improved enough where this feature may no longer be necessary.

To be clear, the proposal is _not_ to remove linting in tests. It is to follow
the rest of JavaScript community and follow the standard tooling process.

There are multiple ways to run `eslint`:

1. Integration with editors
2. Utilize precommit hooks with `eslint`
3. Support a standard way to run `eslint` (such as `yarn lint:js`)

We can also discuss configuring `testem` to automatically run `eslint` as part
of `yarn test`

# Motivation

1. Improve our build speed
2. Simplicity. `eslint` is common among JS stack, and integrations with editors
/ precommit-hooks are ubiquitous. Removing this layer of abstraction will
simplify how `eslint` is used throughout `ember-cli`. Most editors have
plugins available for `eslint`, and as long as the `.eslint.rc` is not
removed, we should still see the benefits of `eslint` in our Ember projects.
3. Hacks required to support features such as [PR #122
broccoli-lint-eslint](https://github.com/ember-cli/broccoli-lint-eslint/pull/122#discussion-diff-153937455R28)

# Detailed design

1. Change blueprint to pull in `eslint` as opposed to `ember-cli-eslint` under
`devDependencies`.
2. Provide documentation on `eslint` and editor integration as well as precommit hooks

Redefine `npm test` or `yarn test` (depending on whether the `--yarn` option was
used to create project) to

```
ember test && npm run lint:js && npm run lint:hbs
```

and

```
ember test && yarn lint:js && yarn lint:hbs
```


# How We Teach This

Providing documentation regarding how to run linting should suffice as well as
documentation to editor integration.

Deleting abstractions and going towards a explicit path, `eslint` within the
`ember-cli` ecosystem becomes _easier_ to teach.

# Drawbacks

1. No console warnings during builds
2. lint failures are no longer included in browser tests

# Alternatives

1. Leave `ember-cli-eslint` alone

# Unresolved questions

N/A

0 comments on commit 72baee5

Please sign in to comment.