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 linting action #1694

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add linting action #1694

wants to merge 5 commits into from

Conversation

alastair
Copy link
Member

@alastair alastair commented Sep 19, 2023

Description
This adds a sample flake8 linting check to freesound. It's based on a generic linter setup that I have in some other projects.
I'm just interested in seeing what kind of errors are reported - it's possible that we might decide that all of these tests are terrible and not really useful. Let's see what happens and if the kind of reports are useful. We can remove checks as necessary if we don't like them, or consider looking at other flak8 plugins to see if we want to add more

I added a basic config file, but we should also consider moving it to a pyproject.toml

It would be nice to use github's built in pip cache to speed up installation.

I've previously had problems with this action, as there are no versions specified in the requirements file. So if a new version introduces new checks, suddenly an unrelated change will result in a bunch of failures. We should consider how we define our dependencies (in a separate linting file, or as part of the dev dependencies?), and decide how we want to specify versions.

@alastair
Copy link
Member Author

something I remembered here -
the flake8 linter also checks that the order of imports is "correct" - that they are grouped in stdlib/dependencies/application and that within these groups they're alphabetical.
We should use isort to automatically sort these (perhaps at the same step as yapf), so that we don't need to manually go and fix ordering after the test fails

@alastair alastair force-pushed the linting branch 2 times, most recently from 73395bf to 5ab3aac Compare December 20, 2023 11: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.

1 participant