-
Notifications
You must be signed in to change notification settings - Fork 654
Run django-upgrade and linting on CI #2820
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
base: main
Are you sure you want to change the base?
Conversation
| rev: 1.29.1 | ||
| hooks: | ||
| - id: django-upgrade | ||
| args: [--target-version=4.2] |
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.
The tool looks for the Django version in pyproject.toml. We don't have one, so defaults to 2.2. So let's specify it here, and remember to update when also bumping the Django version in base-requirements.txt.
https://django-upgrade.readthedocs.io/en/latest/options.html#cmdoption-target-version
| ci: | ||
| autoupdate_schedule: quarterly |
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.
In case we want to enable https://pre-commit.ci/ ? Be less spammy than the default weekly.
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.
pre-commit.ci would also replace your github action and run faster. It's quite optimized
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.
I think it's also useful to have it in GitHub Actions, to be able to already catch things in forks.
| re_path(r'^funding.json$', views.serve_funding_json, name='funding_json'), | ||
| path('humans.txt', TemplateView.as_view(template_name='humans.txt', content_type='text/plain')), | ||
| path('robots.txt', TemplateView.as_view(template_name='robots.txt', content_type='text/plain')), | ||
| path('funding.json', views.serve_funding_json, name='funding_json'), |
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.
More about this one: https://django-upgrade.readthedocs.io/en/latest/fixers.html#urls
| return {'slug': ['name']} | ||
| return {} | ||
|
|
||
| @admin.display(description="Revenue split") |
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.
Added in Django 3.2: https://django-upgrade.readthedocs.io/en/latest/fixers.html#admin-display
| featured_stories = Story.objects.featured() | ||
| expected_repr = [f'<Story: {self.story3.name}>'] | ||
| self.assertQuerysetEqual(featured_stories, expected_repr, transform=repr) | ||
| self.assertQuerySetEqual(featured_stories, expected_repr, transform=repr) |
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.
assertQuerySetEqual added in 4.2: https://django-upgrade.readthedocs.io/en/latest/fixers.html#assertformseterror-and-assertquerysetequal
assertQuerysetEqual removed in 5.1: https://docs.djangoproject.com/en/dev/internals/deprecation/#deprecation-removed-in-5-1
Description
django-upgrade upgrades Django code, which can help update deprecations to make upgrading to the next LTS easier.
Also add it to
.pre-commit-config.yamlwith some other linters, and run it on GitHub Actions.We can also add things like Ruff formatting/linting to
.pre-commit-config.yamllater: #2540