Fix eslint for Atom

[marlowpayne]

Status: Ready for Review
Owner: @marlowpayne
Reviewers: @donnielrt @Helen-Mobify

Changes

Atom currently has issues parsing our eslint settings due to improper quote marks in certain settings.

In addition, Atom will not overwrite default eslint settings unless they are explicitly whitelisted in the supplied eslint config. These changes:

• Fix rules that Atom previously could not parse correctly
• Extend our eslint config on top of a blank slate of disabled default rules

These changes are compatible with our usage of eslint in Adaptive.js projects.

Todos:

• Engineer +1

Feedback:

none so far

How to Test

A couple things to test here:

Test on-the-fly linting in Atom

• Download and install Atom
• Install the linter package
• Install the linter-eslint package
• Update your local copy of the code-style by running npm install -g git+ssh://git@github.com:mobify/mobify-code-style.git#fix-eslint-for-atom
• Create a symlink to the updated eslint config files by navigating to your main projects directory (holding all your Mobify projects) and running:

ln -s /path/to/your/global/node_modules/mobify-code-style/javascript/.eslintrc ./.eslintrc
and
ln -s /path/to/your/global/node_modules/mobify-code-style/javascript/.eslintrc-reset ./.eslintrc-reset

(Default path is usually /usr/local/lib/node_modules/)

• Open your Atom settings for the linter-eslint package and make sure your path is set for the eslint rules
  
• Restart Atom
• Open a JS file and check that linting works

Test that project linting still works

• Open an Adaptive.js project built from the generator version 1.8 or higher (uses eslint)
• Update package.json to use this branch's version of eslint config by changing the entry for mobify-code-style to:

"mobify-code-style": "git+ssh://git@github.com:mobify/mobify-code-style.git#fix-eslint-for-atom",

• Refresh the project node modules with: rm -rf node_modules && npm install
• Verify linting runs against the new rules with: grunt lint

[wizardlyhel]

Cool @marlowpayne I'll test this out in near future.

[donnielrt]

@marlowpayne does the inclusion of the default rules mean that we might have more failures than expected? Have you tested this on existing projects?

[marlowpayne]

@donnielrt the default ruleset acts as a 'reset' so that our rules are the only ones taken into account when linting files. We should only get linting errors when one of our rules get broken. I could rename from .eslintrc-defaults to .eslintrc-reset or something to make that more clear.

I have tested this on the newly ported Christie's Adaptive and Via Magazine projects and have not run into any false positive failures (Via needed an update for grunt-eslint, but I think that is out of the scope of these changes).

[donnielrt]

@marlowpayne ok, perfect. The changes look good, but I'd wait for @Helen-Mobify to verify.

👍 from me.

[wizardlyhel]

@marlowpayne

According to the description, the symbolic link you are creating are to each project? or is it to the mobify-code-style repo that we should be pulling down as well?

Should we figure out a way to have Atom to look for the local mobify-code-style in the project instead of a symbolic link to a global one?

[marlowpayne]

@Helen-Mobify the symbolic links are not for individual projects, they are for the parent directory containing all the Mobify projects and codebases. This is so that eslint will initially look for a .eslintrc config in the project directory, then go up the hierarchy and find the symlink in the parent directory. The symlink is for your local globally installed code style node module (using npm install -g mobify-code-style, users shouldn't need a local copy of the repo). Linking it means that all projects can abide by the same code style and it can be updated in one place, with one command (instead of having to update each project individually when a code style update drops).

[donnielrt]

@marlowpayne just to be clear, we don't want one global copy of eslintrc or other code style documents to override the project's lint settings. A project should use its local copy of Mobify Code Style, because different projects might've been written with different versions of the linting files.

It's fine if you just meant this as a backup copy of lint settings if a project config wasn't found.

[wizardlyhel]

Yea that is what I thought as well @donnielrt we download a local copy of mobify-code-style for each project. If it were global, there is no need for that.

[marlowpayne]

@donnielrt right. Sorry, I didn't make it very clear. The symlinks are only there for text editors. They don't have any effect on the individual projects' linting settings (those are encapsulated in their own node modules and the project's grunt lint task only act on those rulesets).

[wizardlyhel]

Okay. I verified it.

The text editor is grabbing the mobify-code-style rules but does not affect the project's own linting file.

👍

[donnielrt]

Sweet 👍

[marlowpayne]

\o/ Merging!