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

Adding support for pre-commit hooks #1282

Closed
javulticat opened this issue Nov 6, 2023 · 1 comment
Closed

Adding support for pre-commit hooks #1282

javulticat opened this issue Nov 6, 2023 · 1 comment
Assignees

Comments

@javulticat
Copy link
Member

Zappa is a project that began development over 8 years ago and still relies heavily on a Makefile (which are pretty outdated in Python projects) and human memory to do local development. With our current setup, I've probably had to remind users to run make black or make isort hundreds of times in Zappa PRs at this point.

There's a better way. pre-commit is now more-or-less the industry standard to ensure that things developers need to run before pushing a commit are run automatically, without the developer needing to remember to do so. I haven't worked a job in probably half a decade that didn't make use of pre-commit in their repos. Setting up pre-commit is on the checklist of "first things to do" when setting up a personal repo or a repo for my engineering team.

The proverbial straw that broke the camel's back for me today is that I was making an extremely minor change to our README, and the tests complained I needed to run doctoc before I could merge my changes. Now, the use of and need for doctoc isn't documented anywhere in the Zappa repo AFAIK, but that's a separate problem for another day.

The main issue I had with that is that doctoc is a Node.js package that wanted me to install it with npm. I didn't have Node.js or npm installed in the environment I was working in. And, imo, it's just absolute insanity that we require contributors to install Node.js, a Node package manager, and a specific node package just to be able to contribute to the damn README of our Python project.

Now, fortunately, like most tools found in the civilized world, doctoc offers a pre-commit hook, which ameliorates the problem, as pre-commit automatically takes care of configuring the necessary environment to run any pre-commit hook you can find. So, no more need for a user wanting to change a line in the README to have to install Node.js, npm, and doctoc on their machine; only pre-commit is needed, which can be installed via pip just like any other Python package.

In fact, every linting tool we use (and several others we probably should be using): black, isort, flake8, mypy, etc., all have useful pre-commit hooks.

So, I configured and successfully tested using pre-commit to run doctoc on Zappa for me, so I'll be adding the rest of our linting tools as well to a pre-commit configuration for the repo. I'll also see about the feasibility of removing/reducing the need for the Makefile, though will likely do so through a separate PR.

TL;DR: pre-commit is coming soon to a Zappa near you!

@javulticat
Copy link
Member Author

Done in #1284

@javulticat javulticat removed the in-progress Development is underway label Jan 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant