Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Discuss] Extend eslint-config-airbnb? #8

Open
dylanpyle opened this issue Sep 19, 2016 · 6 comments
Open

[Discuss] Extend eslint-config-airbnb? #8

dylanpyle opened this issue Sep 19, 2016 · 6 comments

Comments

@dylanpyle
Copy link
Contributor

dylanpyle commented Sep 19, 2016

I've been using eslint-config-airbnb recently — it seems like it matches our rules pretty closely, and it's a whole lot stricter about a lot of things, plus well-maintained as new rules are introduced.

I haven't had much time to keep this curated, and I think it might make sense to just remove the bulk of our custom rules, and just extend that set instead. I haven't noticed many direct contradictions with our code style, but lots of clarifications of cases where we use two equivalent patterns and should just pick one.

Pros:

  • Less long-term maintenance cost on us
  • Less bikeshedding (we're pretty good about this, but could always be better)

Cons:

  • Some possibly-controversial rules
  • Large upfront cost in getting all our code in line
@pashariger
Copy link

Their rules are pretty much the gold standard. 👍 👍 👍 👍 👍

@dylanpyle
Copy link
Contributor Author

For context, one of the things that prompted this was that our current ruleset only works with eslint v1 - the current version is v3, so there would still be a bit of effort involved with getting us back up to the current state of things if we don't go this route.

@Shyp/platform — thoughts on this? This stuff has caused a decent amount of debate in the past, I'd kinda like to just be done with it. Here's the config I'm using for some personal stuff, for reference.

@benbuckman
Copy link
Contributor

@dylanpyle This seems reasonable to me. I'd be curious to see the diff on shyp.com to accommodate the new rules.

@caseycrites
Copy link

🍧

@dylanpyle
Copy link
Contributor Author

dylanpyle commented Sep 20, 2016

@benbuckman Here's the current set of failures — I didn't put in the time/effort to start whitelisting rules:

edit: redacted, contact me for the link

The vast majority are the same few things we can easily add exceptions for (e.g. trailing commas, singlequotes), since we have a big corpus of code that contradicts them, and have intentionally chosen to style stuff that way.

There are some issues related to the babel build process - e.g. the import paths failing, syntax errors parsing the propTypes syntax. For these, we could reintroduce the current 'run it through babel' build step, which is fine but a bit slow — in the slightly longer term I'd like to move away from the >ES2015 feature set altogether though, which would clean those up (we're only using a couple of features, so writing a script/macro to fix those should be straightforward).

That only leaves a few substantive ones - things like the order of imports, not writing class instance methods that don't refer to this, some stylistic issues with arrow functions, etc. This is the kind of stuff I'd love to just defer to their choices on — preferring 'do it a certain way that we may not be 100% sold on' over 'do it both ways, or neither'.

@benbuckman
Copy link
Contributor

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants