-
-
Notifications
You must be signed in to change notification settings - Fork 151
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
Ensure that code quality checks are running #204
Conversation
This adds 3 already existing tox targets to github checks include code quality, dist verification, and qunit tests Because these do not fit in the django-python matrix these are added as separate jobs Getting them passing requires several requirements changes correct tox requirements.txt location update lib versions in that requirements file add deps to quality target, since pylint resolves imports Update tox whitelist to allowlist and include timeout for qunit Fix code quality failures a few whitespace points a warning about invalid related_name remove imports of classes that do not exist in tests DRY edits to Makefile remove pylint rcfile and use CLI options instead substantially changes code quality selections enabled
@@ -352,20 +351,4 @@ def set_up_test_model( | |||
bases=["%s.Pony" % app_label], | |||
) | |||
) | |||
if manager_model: | |||
from .models import FoodManager, FoodQuerySet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FoodManager
does not exist in that module. These were adapted from Django, which explains that. I can't imagine this library needing to test this.
pylint --rcfile=pylintrc example sortedm2m sortedm2m_tests test_project *.py | ||
isort --check-only $(CHECK_DIRS) | ||
pycodestyle $(CHECK_DIRS) | ||
DJANGO_SETTINGS_MODULE=test_project.settings pylint --errors-only --load-plugins pylint_django $(CHECK_DIRS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing the --rcfile
is the most opinionated change here - the problem is that I don't have instructions to re-generate the file and the current file is too old and has tons of options that doesn't work anymore. While I can do pylint --generate-rcfile
, this results in a completely different file from what's there, the diff is so large that it's unhelpful. If I had a set of changes that were previously applied to the file last time it was generated, I could go about re-applying those, but I don't know how to get that.
Because of this, I turned it on in the less-strict mode of --errors-only
.
I see that github actions was running a matrix of commands, but it seems that the matrix didn't account for environments that were not within the parameters varied by the matrix. So if you do
tox list
you can see these 3Enabling those, and diving further into this, I suspect that the transition to github actions left the repo without any quality checks for a while. As such, we had some undefined imports and other interesting things discovered by essentially re-enabling these checks.
Re-enabling the checks took a bit of modernization work as well.