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

Add black and flake8 #174

Merged
merged 10 commits into from
Jun 3, 2024
Merged

Add black and flake8 #174

merged 10 commits into from
Jun 3, 2024

Conversation

tianyizheng02
Copy link
Contributor

See this comment thread in #170 for more details

Add black to pre-commit and GitHub actions for automatic code formatting. Also update black to the latest version, as the old version was breaking due to an ImportError.

Add flake8 to pre-commit and Pipfile dependencies for automatic linting beyond GitHub actions.

Update black from 19.10b0 to 24.4.2. black had been broken due to an
ImportError (see psf/black#2964).
@tianyizheng02
Copy link
Contributor Author

python -m pip install --upgrade pip
pip install flake8 pytest
pip install -r requirements.txt

@nij-patel We're still using pip and requirements.txt in our GitHub Actions workflows when we've already transitioned to using pipenv and Pipfile for our dependencies. Is it OK to update the GitHub workflows to use pipenv as well?

Ran `pipenv requirements --exclude-markers > requirements.txt` to update
requirements.txt with correct dependencies and versions from Pipfile. I
did this primarily to update the black version (because the previous
version was broken with an ImportError), and I might as well update the
rest of the dependencies as well.
Add the --check and --diff flags to black within GitHub Actions
workflows so that the workflows fail if files need formatting. Without
these flags, black will "format" the files and exit without error, which
doesn't actually write to the files during workflows.
@tianyizheng02
Copy link
Contributor Author

black fails as expected, since I haven't yet run black on the codebase. I'll keep the flake8 step of the workflows as is, but I don't think they should just let those 97 warnings pass. We can leave that for possible future work.

Add flags to flake8 and black for pre-commit based on existing flags in
GitHub Actions workflows. This includes selecting for specific flake8
errors and adjusting the max line length. These existing configs are
extremely limited, so they should be broadened to more errors and/or a
more widely accepted max line length.

Also ran black on the entire codebase to satisfy the GitHub workflows.
@tianyizheng02
Copy link
Contributor Author

Codebase passes pre-commit and GitHub Actions workflows with flake8 and black enabled

For future reference, note that flake8 is currently only configured to check for syntax errors and undefined names, which is far too narrow. This will need to be broadened to check for more errors (e.g., unused imports and common antipatterns).

@tianyizheng02 tianyizheng02 marked this pull request as ready for review May 24, 2024 12:55
@tianyizheng02
Copy link
Contributor Author

@RitwikGupta?

@tianyizheng02 tianyizheng02 merged commit 551d8d9 into dev Jun 3, 2024
1 check passed
@tianyizheng02 tianyizheng02 deleted the add-black-flake8 branch June 4, 2024 00:57
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.

2 participants