OpenSourceFellows / amplify-back-end

The API backend for ProgramEquity
https://www.programequity.com/
MIT License
11 stars 0 forks source link

Improve pre-commit hooks #145

Closed JamesMGreene closed 2 years ago

JamesMGreene commented 2 years ago

TL;DR: Update our precommit hook to not just check if our code linting or formatting has errors, but rather actually fix such errors (if possible) for any files that are being modified in the pending git commit.


:warning: Possible unexpected behavior

One caveat that I've seen with using lint-staged is that it has the somewhat peculiar behavior of rolling back fixes made by eslint --fix/prettier --write IF those changes resulted in zero staged files remaining -- whereas if there is at least one other file still staged, the commit will go through as expected.

You can work around this behavior by adding the --allow-empty flag to the lint-staged call but then it creates empty commits instead, which also isn't great...? πŸ€” That said, I'd be willing to add that flag if folks would prefer the better usage experience over the cleaner git history. πŸ€·πŸ»β€β™‚οΈ

I've submitted an issue to the lint-staged folks to consider for a behavior that feels less confusing to me: https://github.com/okonet/lint-staged/issues/1078

That said, I don't feel like this shortcoming should prevent us from using this solution as an attempted commit of files whose only changes are bad linting or formatting should definitely be an unlikely event. πŸ˜† πŸ˜…

JamesMGreene commented 2 years ago

Putting this back into Draft for now as the --no-stash has an interesting tradeoff in behavior that I'm not sure I want to introduce.

JamesMGreene commented 2 years ago

Opened an issue in the new repo to reinvestigate this when time allows: https://github.com/ProgramEquity/amplify/issues/80