brave / browser-laptop

[DEPRECATED] Please see https://github.com/brave/brave-browser for the current version of Brave
https://www.brave.com
Other
7.95k stars 975 forks source link

Use own .eslintrc instead of standard #273

Closed craigmichaelmartin closed 8 years ago

craigmichaelmartin commented 8 years ago

Standard is built atop eslint and offers a set of default (unchangeable) eslint configuration rules and pre-commit linting. While this is great for getting off the runway with some sane best practices, it locks you into a linting style you cannot own. Would there be interest in losing the overhead and stripping down to eslint? Initially, we could use the same rules standard chose.

That said, I would already like to propose the prefer-const :) If you are of one mind with standard and see this as bikeshedding, feel free to close!

bbondy commented 8 years ago

You make some good arguments but I'd rather sit on this and leave the issue open, and not act on it right now. Although we should prefer const, we need to stay efficient and can't spend time (right now anyway) being in the business of tweaking / maintaining config files across projects. Just my opinion, others can weigh in too pls :)

craigmichaelmartin commented 8 years ago

That makes a lot of sense. To the point of maintaining / tweaking configs across multiple projects, we could create one shareable config that each projects references.

bbondy commented 8 years ago

Sounds more reasonable if it was an easy install npm published module like standard is. BTW we used to use eslint everywhere and changed to standard for easy setup and less maintaining of style. But if we build up a big enough reason to (so far prefer'ing const) then I'm open to it. As long as it could be published and installed as an npm module with minor tweak to package.json.

diracdeltas commented 8 years ago

maybe my ex-coworker @feross has thoughts here :)

craigmichaelmartin commented 8 years ago

Feross has already articulated his philosophy on this (and even specifically on prefer-const). I happen to agree with his philosophy in general for projects getting off the runway, but not for maturing projects. If you disagree, feel free to close :)

To the 'building up a case for owning our own configurations', I'd like to add: no-var, prefer-arrow-callback, prefer-template, prefer-spread, and object-shorthand.

craigmichaelmartin commented 8 years ago

There is project similar to standard, xo by sindresorhus, that already handles esnext type rules, but again, I see the value in a mature project owning it. If it is done the right way it is a single sharable config that is an easy install npm published module.

bbondy commented 8 years ago

OK let's sit on this a while longer and leave this open. Please collect rule preferences here and we can decide to switch later. We need to focus on the actual project objectives for now. Thanks for the extra rules.

feross commented 8 years ago

The next version of standard (v6, due shortly) will allow you to modify rules on a one-off basis by adding this to package.json:

"standard": {
  "rules": {
    "prefer-const": 2
  }
}

Extending the eslint shareable config is cool too :)

bbondy commented 8 years ago

++ awesome, thanks @feross

craigmichaelmartin commented 8 years ago

:+1: I can be on the look-out for its release and make the rule changes proposed above, if you would like. I'd be happy to take the burden off you, while pushing the javascript standards forward.

diracdeltas commented 8 years ago

@craigmichaelmartin that would be excellent, thank you!

craigmichaelmartin commented 8 years ago

So, standard v6 was released, and with the ability to make rules configurable! However, the owner quickly dis-espoused this change, and plans a quick v7 release to do away with it. As I've advocated, I believe the right approach is a sharable eslint config. Anyway, we can take advantage of v6 for our purposes. Look out for some PRs :)

Flet commented 8 years ago

Another option is to publish a shareable config and use standard-engine to wrap it up in package like standard.

This is how semistandard does it! eslint-config-semistandard mutates a couple standard rules then its wrapped up in the engine for ease of use.

craigmichaelmartin commented 8 years ago

Yeah, I think that is another reasonable path. I intended to make some PRs for this, but am a bit bogged down with some side projects right now and on the sidelines for this. If you, or anyone, have the time, feel free to run with it!

bbondy commented 8 years ago

We're going to stick with standard. Closing.