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 has a style and I just think where possible should enforce that by automation. > AFAIK it does not even install with mach bootstrap. It has worked for me for quite some time just running ./mach lint filename.js after bootstrap. Have a great holiday period too Thanks Jonathan On Sun, Dec 24, 2017 at 7:41 PM, Ben Kelly <bke...@mozilla.com> wrote: > Hello, > > First let me say, I don't like participating in style discussions on the > list. I don't think they are productive in general and I don't want to > have one of those threads here. > > I feel I need to raise as an issue, though, as eslint rules are being > pushed out into components with what seem like overly restrictive style > rules. > > I have no problem with eslint rules that check for language safety issues. > For example, verifying that there are block braces on single line > if-statements has safety value. Verifying that semi-colons are used has > safety values. These are things that can break when new code is added to > the file. > > But I also see rules about cosmetic things like what kind of quotes must be > used for strings. For example: > > https://bugzilla.mozilla.org/show_bug.cgi?id=1423841 > > 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 the author and reviewer of a component prefer to use > single quotes instead of double quotes in a file. > > I also find this kind of cosmetic rule especially frustrating because we do > not run eslint by default. AFAIK it does not even install with mach > bootstrap. If we are going to enforce this kind of thing these checks > should be run as part of a build or when executing tests. They should not > require manual installation of tools and running separate commands. > > As a further anecdote, I have also had to fight eslint rules in devtools > which required things like simultaneously using an extreme indent value and > not exceeding a line width limit. Basically conflicting and impossible > rules to satisfy. I ended up wasting close to an hour figuring out an ugly > work around for the problem. It did not feel like the lint was providing > value. > > Anyway, I just want to make a plea that we don't push out overly > restrictive eslint rules for cosmetic reasons. We should be spending our > time building the browser and not trying to do gymnastics to meet some > arbitrary set of cosmetic rules. Please focus eslint rules on things that > provide code safety value. > > Thanks. Have a great holiday. > > Ben > _______________________________________________ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
