Re: overly strict eslint rules

2018-01-05 Thread Andrew Halberstadt
Not replying to anyone specifically, but Sylvestre's team is working on getting our linters hooked up to mozreview/phabricator as well. I think this paired with bootstrapping the hooks will smooth out the lint fixing workflow considerably. Andrew On Fri, Jan 5, 2018 at 5:58 AM Mark Banner wrote

Re: overly strict eslint rules

2018-01-05 Thread Mark Banner
On 03/01/2018 14:57, Mark Banner wrote: On 24/12/2017 22:07, Masatoshi Kimura wrote: --- $ mach lint browser/base/content/aboutDialog.js eslint-plugin-html v2.0.3 should be v4.0.0. ESLint is an old version, clobbering node_modules direc

Re: overly strict eslint rules

2018-01-03 Thread Mark Banner
On 24/12/2017 22:07, Masatoshi Kimura wrote: I got the following error when I tried it just now: As far as I know, it should work fine with the latest version of MozillaBuild. If it doesn't, please file a bug in Testing:Lint and we'll take a look. --

Re: overly strict eslint rules

2018-01-03 Thread Alex Gaynor
On Wed, Jan 3, 2018 at 4:43 AM, Mark Banner wrote: > On 24/12/2017 19:41, Ben Kelly wrote: > >> But I also see rules about cosmetic things like what kind of quotes must >> be >> used for strings. >> AFAICT this kind of rule does not have any tangible safety benefit. Its >> purely a cosmetic styl

Re: overly strict eslint rules

2018-01-03 Thread Mark Banner
On 24/12/2017 19:41, Ben Kelly wrote: But I also see rules about cosmetic things like what kind of quotes must be used for strings. AFAICT this kind of rule does not have any tangible safety benefit. Its purely a cosmetic style choice. I don't see why we should bounce patches out of the tree if

Re: overly strict eslint rules

2017-12-27 Thread Jeff Gilbert
It's not just eslint either. We warn in static analysis now when using `!std::vector::size()` instead of `empty()`. Such over-prescriptive linting is unnecessary and unproductive. On Sun, Dec 24, 2017 at 2:07 PM, Masatoshi Kimura wrote: > > On 2017/12/25 6:31, Jonathan Kingston wrote: >> It has w

Re: overly strict eslint rules

2017-12-24 Thread Masatoshi Kimura
On 2017/12/25 6:31, Jonathan Kingston wrote: > It has worked for me for quite some time just running ./mach lint > filename.js after bootstrap. I got the following error when I tried it just now: --- $ mach lint browser/base/content/abo

Re: overly strict eslint rules

2017-12-24 Thread Jonathan Kingston
I'm not sure on the exact rule that was failing. However having failed code reviews that would pass in one part of the codebase can be pretty frustrating. I would rather stylistic errors come from automated tests and such that checking of algos and so on can be focused on in a code review. Firefox