parse-dashboard icon indicating copy to clipboard operation
parse-dashboard copied to clipboard

Add ESLint to CI and Commit Hooks

Open dblythy opened this issue 4 years ago • 4 comments

New Feature / Enhancement Checklist

Current Limitation

Currently, there is no ESLint enforcement on this repo. Nor is there an option to run npm run lint-fix like the other repo's have.

Feature / Enhancement Description

Having lint as part of our CI here would help detect bugs and poorly written code.

Example Use Case

Alternatives / Workarounds

Manually run lint

3rd Party References

dblythy avatar Sep 05 '21 13:09 dblythy

Thanks for opening this issue!

  • 🎉 We are excited about your ideas for improvement!

I personally am not particularly fond of commit hooks. I think we have discussed that in the past - a prettified code block may not always be the most practical style during code development. I would prefer a CI step over commit hooks. Unless having both brings any benefits?

mtrezza avatar Oct 09 '21 12:10 mtrezza

I think commit hooks are good to make sure that PRs fit the lint standards that we apply, regardless of the additional styles of prettier. Hooks can always be skipped with --no-verify

dblythy avatar Oct 09 '21 12:10 dblythy

As I understand it, "ensuring" would be the purpose of the CI step "adapting" would be the purpose of the commit hook

I don't believe it's developer friendly to force-prettify code they are still working on when making a commit. Just thinking about breakpoints that are set to specific lines, when suddenly the lines change because of a prettify. When working on a code block and the structure suddenly changes, that could also be somewhat disruptive as the developer would have to reorient visually. A developer would have to move to use git shell commands to use --no-verify because a git UI (like GitHub Desktop) rarely supports this flag.

When would these hooks be invoked?

mtrezza avatar Oct 09 '21 13:10 mtrezza