Skip to content

Instantly share code, notes, and snippets.

@MrJadaml
Last active February 14, 2019 21:18
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save MrJadaml/0da210ab44f7f081c932d8b4bf1617ea to your computer and use it in GitHub Desktop.
Save MrJadaml/0da210ab44f7f081c932d8b4bf1617ea to your computer and use it in GitHub Desktop.

Summary

Adoption of AirBnb Style Guide + Lint Config.

Motivation

Currently we have a linter config, but don't keep it in sync with the rules defined in our style guide. This requires devs to regularly reference the style guide. More realistically speaking, PRs are submitted with code that does not comply with our style guide for rules that are not also defined in our config -- both new and seasoned devs do this. This causes us to kick back PRs due to code style violations which adds additional PR review cycles consuming multiple developer's time. These review cycles are slow and tedious and could be avoided if the config were kept up to date. Having an up-to-date config would also allow us to hook in and reject push/commits if the linter fails, saving several devs time. Likewise we could validate linting through travis and have it fail if is does not pass again saving devs time.

Adoption of AirBnb's code styles would alleviate us from having to maintaining a style guide. We would benefit from inheriting a very well documented style guide that provides clear descriptions and examples behind the value of each rule. The effort required to maintain and replicate the quality of AirBnb's documentation feels like a low ROI.

We would reduce dev time spent maintaining linter config and style guide allowing us to spend more time on work that provides a higher rate of return. AirBnb's guide and config has greater coverage, encompassing more code patterns which would ensure less variations on code styles and enforce a wider breadth of consistency across the code bases.

It's an industry standard -- One of the most widely adopted JS & React style guides -- just google "JS style guide". By the nature of it being an industry standard, new hires would likely to be familiar with it which would reduce friction during ramp-up. This would push us one step closer to our goal of working within a CI/CD pipeline.

The expected outcome will be faster turnaround time on tickets and reduced developer frustrations.

Ultimately the motivation here is to expedite having standardize code formatting rules implemented and automated in the codebase so that more time can be allocated to work with higher business value. The AirBnb config could be up and running in less than 3min.

Detailed design

The AirBnb Lint Config takes just a couple min to set up.

Eslint comes with a cli that with a setup wizard -- eslint --init that provides AirBnb as one of their default setup options. It will even ask if you are using React or not to provide a more tailored setup. This automated setup would be useful when it come to all the mirco-apps we plan to be spinning out and keeping them in sync in terms of linter configurations.

npx install-peerdeps --dev eslint-config-airbnb

Update our .eslintrc

---
parser: babel-eslint
extends: airbnb           // UPDATED
env:
  browser: true
  node: true
  mocha: true            // COULD BE REMOVED
  es6: true
  jest: true
ecmaFeatures:
  modules: true
  jsx: true
globals:
  ENV: true
plugins:
- standard
- jest
- react
rules:

This just sets the airbnb config as our "base" config, meaning we can still easily extend it with our own rules. I would however suggest maybe removing all the currently defined rules and just introduce a few back in if we have strong push back from the team on any of the airbnb defaults.

run the linter

yarn lint

Drawbacks

Adoption will create thousands of linter errors/warnings that will need to be addressed.

Alternatives

Update current .eslintrc to match all of the styles defined in our current style guide

  • This will also include looking up the corresponding values for the rule to reflect how we want that rule to function. _ This will also trigger lots of errors that will need to be addressed -- admitted not as many though.

The impact of not doing this would be more dev time spent either maintaining a style guide and lint config, or more time spent going back and forth on PR reviews. Also would mean more inconsistency in the code base leading to slower dev cycles and ticket turnaround time.

Adoption strategy

We could take an incremental strategy similar to how we approach "clean-up" in areas of the codebase we touch. There could also be an initial cleanup of low risk updates with areas like tests and other dev modules. There could then be a similar round in /src only tackling low risk updates -- things like undeclared defs, whitespace rules etc. There could then be a "wrap-up" phase down the line where it is clear the dependency requirement on the exiting pattern is mostly gone.

There should be no breaking changes or coordination required with other projects or libraries.

How we teach this

AirBnb provides excellent documentation for their Style Guide here. In addition you can see their open source Lint Config here.

Since this is a very well known and widely used tool, many devs are already familiar with it.

Unresolved questions

What does it look like to slide this over to other frameworks so that we can have a "company" pattern rather than an "app" pattern?

  • JS specific -- framework specific styles would obvi only apply to a single app

What other adoption strategies should be considered here?

@garrettmac
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment