From aa12a5f967705f70b1dbe457bb2396d106e3570b Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Mon, 5 Apr 2021 12:08:52 +0100 Subject: [PATCH] Lint with pre-commit (#7900) Following [my comment here](https://github.com/encode/django-rest-framework/pull/7589#issuecomment-813301322) 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. --- .github/workflows/pre-commit.yml | 24 +++++++++ .gitignore | 3 +- .pre-commit-config.yaml | 20 +++++++ .travis.yml | 1 - docs/community/contributing.md | 22 ++++---- requirements.txt | 1 - requirements/requirements-codestyle.txt | 6 --- runtests.py | 70 +------------------------ tox.ini | 12 ++--- 9 files changed, 59 insertions(+), 100 deletions(-) create mode 100644 .github/workflows/pre-commit.yml create mode 100644 .pre-commit-config.yaml delete mode 100644 requirements/requirements-codestyle.txt diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml new file mode 100644 index 000000000..9c29ed056 --- /dev/null +++ b/.github/workflows/pre-commit.yml @@ -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 }} diff --git a/.gitignore b/.gitignore index 82e885ede..7cb1eb249 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ MANIFEST coverage.* +!.github !.gitignore +!.pre-commit-config.yaml !.travis.yml -!.isort.cfg diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 000000000..0fc181b10 --- /dev/null +++ b/.pre-commit-config.yaml @@ -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 diff --git a/.travis.yml b/.travis.yml index 57a91e594..244ab77fa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -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" diff --git a/docs/community/contributing.md b/docs/community/contributing.md index cb67100d2..e220f95fc 100644 --- a/docs/community/contributing.md +++ b/docs/community/contributing.md @@ -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 @@ -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 diff --git a/requirements.txt b/requirements.txt index b4e5ff579..395f3b7a8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -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 diff --git a/requirements/requirements-codestyle.txt b/requirements/requirements-codestyle.txt deleted file mode 100644 index d9a93884c..000000000 --- a/requirements/requirements-codestyle.txt +++ /dev/null @@ -1,6 +0,0 @@ -# PEP8 code linting, which we run on all commits. -flake8>=3.8.4,<3.9 -flake8-tidy-imports>=4.1.0,<4.2 - -# Sort and lint imports -isort>=5.6.2,<6.0 diff --git a/runtests.py b/runtests.py index 82028ea32..c340b55d8 100755 --- a/runtests.py +++ b/runtests.py @@ -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) @@ -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] @@ -104,14 +45,5 @@ if __name__ == "__main__": # `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)) diff --git a/tox.ini b/tox.ini index df16cf947..fc44b52d2 100644 --- a/tox.ini +++ b/tox.ini @@ -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 = @@ -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 @@ -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