Skip to content

Conversation

@kelsonpw
Copy link
Contributor

@kelsonpw kelsonpw commented Dec 17, 2020

Summary

This PR adds a few packages:
prettier - for code formatting
eslint - for code quality and formatting
husky - for running prettier and eslint based on a git hook
lint-staged - runs prettier and eslint against staged files on the pre-commit hook

This PR removes a package:
jshint - outdated js code quality tool which has been long replaced by eslint, I removed any jshint related code.

It also adds two new tests to test.yml, one for prettier check and one for eslint check, test will fail if either dont pass. proof (had to push with --no-verify to skip commit hooks):
image

I also copied over the .vscode folder from Amplitude-Node, removing any TS specific stuff

Checklist

  • Does your PR title have the correct title format?
  • Does your PR have a breaking change?: No

@kelsonpw kelsonpw changed the title [style/feat] Add prettier and eslint to run on commit and build, deprecate jshint style(automatting and quality): Add prettier and eslint to run on commit and build, deprecate jshint Dec 17, 2020
@kelsonpw
Copy link
Contributor Author

@haoliu-amp @jooohhn @kelvin-lu

How do you feel about using lint-staged to run prettier and eslint against every commit? I've found it to be very helpful but if the team consistently uses VScode we can also just utilize the autosave/format config from that.

@kelvin-lu
Copy link
Contributor

think it's a good idea since other contributors might not be using vscode and can introduce lint errors (which I've seen @jooohhn catch out of diligence - but it would be better if this was automated.)

@kelsonpw kelsonpw force-pushed the AMP-32165-add-prettier-eslint branch from 950c48f to ac78975 Compare December 17, 2020 23:59
@kelsonpw kelsonpw force-pushed the AMP-32165-add-prettier-eslint branch from ac78975 to 2366868 Compare December 22, 2020 00:48
@kelsonpw kelsonpw force-pushed the AMP-32165-add-prettier-eslint branch from fd2822d to f8ab2d5 Compare December 22, 2020 01:10
@kelsonpw kelsonpw force-pushed the AMP-32165-add-prettier-eslint branch from f8ab2d5 to 1dcc59b Compare December 22, 2020 01:15
Copy link
Contributor

@kelvin-lu kelvin-lu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! will defer to @jooohhn though for another review

Copy link
Contributor

@jooohhn jooohhn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @kelsonpw

@kelsonpw kelsonpw merged commit 79585d5 into master Dec 22, 2020
@kelsonpw kelsonpw deleted the AMP-32165-add-prettier-eslint branch December 22, 2020 16:27
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

Successfully merging this pull request may close these issues.

4 participants