Currently mach eslint only check in devtools, loop and android. We want to extend that to be available to the whole tree.

Comment on attachment 8693063 [details] [diff] [review] Create a top-level eslintignore file Review of attachment 8693063 [details] [diff] [review]: ----------------------------------------------------------------- Forwarding to mconley because I need to run out, but looks mostly OK to me. One thing to note is that this will auto-enable new directories. Because negations don't work I don't think there's a way around this, but it would be good to announce this publicly because it will catch people out if/when we start enforcing this. ::: .eslintignore @@ +1,5 @@ > +# Always ignore node_modules. > +**/node_modules/**/*.* > + > +# Exclude expected objdirs. > +obj/** This should probably be more generic, like obj*/** or something? By default the objdir will be obj-@CONFIG_GUESS@ which won't match this.

(In reply to :Gijs Kruitbosch from comment #2) > Comment on attachment 8693063 [details] [diff] [review] > Create a top-level eslintignore file > > Review of attachment 8693063 [details] [diff] [review]: > ----------------------------------------------------------------- > > Forwarding to mconley because I need to run out, but looks mostly OK to me. > > One thing to note is that this will auto-enable new directories. Because > negations don't work I don't think there's a way around this, but it would > be good to announce this publicly because it will catch people out if/when > we start enforcing this. I don't think we can avoid this as you day but it could be a good thing if once we're enforcing any new code people write has to pass the linting rules or be rejected from the tree ;) > ::: .eslintignore > @@ +1,5 @@ > > +# Always ignore node_modules. > > +**/node_modules/**/*.* > > + > > +# Exclude expected objdirs. > > +obj/** > > This should probably be more generic, like obj*/** or something? By default > the objdir will be obj-@CONFIG_GUESS@ which won't match this. Good catch

Comment on attachment 8693063 [details] [diff] [review] Create a top-level eslintignore file Review of attachment 8693063 [details] [diff] [review]: ----------------------------------------------------------------- Stoked for robot overlords. ::: .eslintignore @@ +58,5 @@ > +# browser/ exclusions > +browser/app/** > +browser/base/** > +browser/branding/** > +browser/components/about/** As mentioned in IRC, I don't think this will ignore things directly under browser/components, like nsBrowserGlue.js. Same probably goes for any JS files in other subfolders where we're doing sub-subfolder ignores. @@ +103,5 @@ > +# Libs we don't need to check > +browser/components/loop/content/libs > +browser/components/loop/content/shared/libs > +browser/components/loop/standalone/content/libs > +browser/components/loop/standalone/node_modules Is this not redundant with the **/node_modules/**/*.* rule up top? @@ +108,5 @@ > +# Libs we don't need to check > +browser/components/loop/test/shared/vendor > +# Coverage files > +browser/components/loop/test/coverage > +browser/components/loop/test/node_modules Also redundant?

This adds a minimal .eslintrc globally, and then enables one rule for browser: eol-last. As a side effect of turning on es6, it also requires that "yield"s are in functions marked as generators (which our parser has allows not to happen atm). We can turn on more rules/directories as we go. Mossop gave me r+ over irc.

Comment on attachment 8693116 [details] [diff] [review] Add a minimal .eslintrc configuration for browser and start linting a few browser files with basic rules. Review of attachment 8693116 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/experiments/Experiments.jsm @@ +143,5 @@ > // OS.File.read. > // Returns a Promise resolved with the json payload or rejected with > // OS.File.Error or JSON.parse() errors. > function loadJSONAsync(file, options) { > + return Task.spawn(function*() { This change is breaking xpcshell tests on fx-team. |Task.Result| isn't supported with star functions. You need to change that throw to a return.

Backed out in https://hg.mozilla.org/integration/fx-team/rev/40981d27ace0 since I needed to see xpcshell without this failing.

(In reply to Kris Maglione [:kmag] from comment #8) > This change is breaking xpcshell tests on fx-team. |Task.Result| isn't > supported with star functions. You need to change that throw to a return. Oops, sorry about that, I've fixed it and re-pushed.