Skip to content

Commit

Permalink
Lint with pre-commit (#7900)
Browse files Browse the repository at this point in the history
Following [my comment here](#7589 (comment)) and [Django's own move to pre-commit](https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/coding-style/#pre-commit-checks).

* Add pre-commit config file to run flake8 and isort.
* Add extra "common sense" hooks.
* Run pre-commit on GitHub actions using the [official action](https://github.com/pre-commit/action/). This is a good way to get up-and-running but it would be better if we activated [pre-commit.ci](https://pre-commit.ci/), which is faster and will auto-update the hooks for us going forwards.
* Remove `runtests.py` code for running linting tools.
* Remove `runtests.py --fast` flag, since that would now just run `pytest -q`, which can be done with `runtests.py -q` instead.
* Remove tox configuration and requirements files for linting.
* Update the contributing guide to mention setting up pre-commit.
  • Loading branch information
adamchainz authored Apr 5, 2021
1 parent 846fe70 commit aa12a5f
Show file tree
Hide file tree
Showing 9 changed files with 59 additions and 100 deletions.
24 changes: 24 additions & 0 deletions .github/workflows/pre-commit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
name: pre-commit

on:
push:
branches:
- master
pull_request:

jobs:
pre-commit:
runs-on: ubuntu-20.04

steps:
- uses: actions/checkout@v2
with:
fetch-depth: 0

- uses: actions/setup-python@v2
with:
python-version: 3.9

- uses: pre-commit/action@v2.0.0
with:
token: ${{ secrets.GITHUB_TOKEN }}
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
MANIFEST
coverage.*

!.github
!.gitignore
!.pre-commit-config.yaml
!.travis.yml
!.isort.cfg
20 changes: 20 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v3.4.0
hooks:
- id: check-added-large-files
- id: check-case-conflict
- id: check-json
- id: check-merge-conflict
- id: check-symlinks
- id: check-toml
- repo: https://github.com/pycqa/isort
rev: 5.8.0
hooks:
- id: isort
- repo: https://gitlab.com/pycqa/flake8
rev: 3.9.0
hooks:
- id: flake8
additional_dependencies:
- flake8-tidy-imports
1 change: 0 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ matrix:
- { python: "3.9", env: DJANGO=main }

- { python: "3.8", env: TOXENV=base }
- { python: "3.8", env: TOXENV=lint }
- { python: "3.8", env: TOXENV=docs }

- python: "3.8"
Expand Down
22 changes: 9 additions & 13 deletions docs/community/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,19 @@ To start developing on Django REST framework, first create a Fork from the
Then clone your fork. The clone command will look like this, with your GitHub
username instead of YOUR-USERNAME:

git clone https://github.com/YOUR-USERNAME/Spoon-Knife
git clone https://github.com/YOUR-USERNAME/django-rest-framework

See GitHub's [_Fork a Repo_][how-to-fork] Guide for more help.

Changes should broadly follow the [PEP 8][pep-8] style conventions, and we recommend you set up your editor to automatically indicate non-conforming styles.
You can check your contributions against these conventions each time you commit using the [pre-commit](https://pre-commit.com/) hooks, which we also run on CI.
To set them up, first ensure you have the pre-commit tool installed, for example:

python -m pip install pre-commit

Then run:

pre-commit install

## Testing

Expand All @@ -79,18 +87,6 @@ Run using a more concise output style.

./runtests.py -q

Run the tests using a more concise output style, no coverage, no flake8.

./runtests.py --fast

Don't run the flake8 code linting.

./runtests.py --nolint

Only run the flake8 code linting, don't run the tests.

./runtests.py --lintonly

Run the tests for a given test case.

./runtests.py MyTestCase
Expand Down
1 change: 0 additions & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,4 @@
-r requirements/requirements-optionals.txt
-r requirements/requirements-testing.txt
-r requirements/requirements-documentation.txt
-r requirements/requirements-codestyle.txt
-r requirements/requirements-packaging.txt
6 changes: 0 additions & 6 deletions requirements/requirements-codestyle.txt

This file was deleted.

70 changes: 1 addition & 69 deletions runtests.py
Original file line number Diff line number Diff line change
@@ -1,42 +1,8 @@
#! /usr/bin/env python3
import subprocess
import sys

import pytest

PYTEST_ARGS = {
'default': [],
'fast': ['-q'],
}

FLAKE8_ARGS = ['rest_framework', 'tests']

ISORT_ARGS = ['--check-only', '--diff', 'rest_framework', 'tests']


def exit_on_failure(ret, message=None):
if ret:
sys.exit(ret)


def flake8_main(args):
print('Running flake8 code linting')
ret = subprocess.call(['flake8'] + args)
print('flake8 failed' if ret else 'flake8 passed')
return ret


def isort_main(args):
print('Running isort code checking')
ret = subprocess.call(['isort'] + args)

if ret:
print('isort failed: Some modules have incorrectly ordered imports. Fix by running `isort --recursive .`')
else:
print('isort passed')

return ret


def split_class_and_function(string):
class_string, function_string = string.split('.', 1)
Expand All @@ -54,31 +20,6 @@ def is_class(string):


if __name__ == "__main__":
try:
sys.argv.remove('--nolint')
except ValueError:
run_flake8 = True
run_isort = True
else:
run_flake8 = False
run_isort = False

try:
sys.argv.remove('--lintonly')
except ValueError:
run_tests = True
else:
run_tests = False

try:
sys.argv.remove('--fast')
except ValueError:
style = 'default'
else:
style = 'fast'
run_flake8 = False
run_isort = False

if len(sys.argv) > 1:
pytest_args = sys.argv[1:]
first_arg = pytest_args[0]
Expand All @@ -104,14 +45,5 @@ def is_class(string):
# `runtests.py TestCase [flags]`
# `runtests.py test_function [flags]`
pytest_args = ['tests', '-k', pytest_args[0]] + pytest_args[1:]
else:
pytest_args = PYTEST_ARGS[style]

if run_tests:
exit_on_failure(pytest.main(pytest_args))

if run_flake8:
exit_on_failure(flake8_main(FLAKE8_ARGS))

if run_isort:
exit_on_failure(isort_main(ISORT_ARGS))
sys.exit(pytest.main(pytest_args))
12 changes: 3 additions & 9 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ envlist =
{py36,py37,py38,py39}-django31,
{py36,py37,py38,py39}-django32,
{py38,py39}-djangomain,
base,dist,lint,docs,
base,dist,docs,

[travis:env]
DJANGO =
Expand All @@ -16,7 +16,7 @@ DJANGO =
main: djangomain

[testenv]
commands = python -W error::DeprecationWarning -W error::PendingDeprecationWarning runtests.py --fast --coverage {posargs}
commands = python -W error::DeprecationWarning -W error::PendingDeprecationWarning runtests.py --coverage {posargs}
envdir = {toxworkdir}/venvs/{envname}
setenv =
PYTHONDONTWRITEBYTECODE=1
Expand All @@ -37,18 +37,12 @@ deps =
-rrequirements/requirements-testing.txt

[testenv:dist]
commands = ./runtests.py --fast --no-pkgroot --staticfiles {posargs}
commands = ./runtests.py --no-pkgroot --staticfiles {posargs}
deps =
django
-rrequirements/requirements-testing.txt
-rrequirements/requirements-optionals.txt

[testenv:lint]
commands = ./runtests.py --lintonly
deps =
-rrequirements/requirements-codestyle.txt
-rrequirements/requirements-testing.txt

[testenv:docs]
skip_install = true
commands = mkdocs build
Expand Down

0 comments on commit aa12a5f

Please sign in to comment.