michelmansour / virgil

A delightful way to share, discuss, and annotate poetry
MIT License
2 stars 2 forks source link

Add eslint #3

Closed michelmansour closed 8 years ago

michelmansour commented 8 years ago

This PR introduces eslint and the AirBnB ruleset for Javascript/React. Some other small changes are also included.

@drewrey All rules are open to discussion, now and in the future. I'd like to get your ok before merging.

michelmansour commented 8 years ago

Also, @drewrey, since there are so many changes, it would be great if you could check out this branch locally and test the app to make sure everything still works as expected.

drewrey commented 8 years ago

Running npm run lint, I get:

Cannot find module 'babel-eslint'

babel-eslint's install instructions use --save-dev, should we follow that? Do most people install javascript linters globally?

There's a note on babel-eslint's README suggesting we be able get by with just eslint (with some config) unless we use:

stuff like class properties, decorators, async/await, types.

I haven't noticed those things, but maybe they're already here, or maybe you intend to make use of them.

The app is working as expected for me. 👍

michelmansour commented 8 years ago

Running npm run lint, I get:

Cannot find module 'babel-eslint'

babel-eslint's install instructions use --save-dev, should we follow that? Do most people install javascript linters globally?

Oh, good catch. Forgot to include it in the dev-dependencies.

There's a note on babel-eslint's README suggesting we be able get by with just eslint (with some config) unless we use:

Yes, I noticed that, too, but it wasn't working for me without babel-eslint. I'll try again.

michelmansour commented 8 years ago

Actually, we are using class properties: defining instance methods using arrow function, so we do need babel-eslint. I'll add it to dev-dependencies.

michelmansour commented 8 years ago

Force pushed with babel-eslint included. Please try again.

drewrey commented 8 years ago

LGTM