-
Notifications
You must be signed in to change notification settings - Fork 722
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
GitHub Action to lint Python code with ruff #718
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,22 @@ | ||
name: lint_python | ||
on: [pull_request, push] | ||
on: | ||
push: | ||
branches: [master] | ||
pull_request: | ||
jobs: | ||
lint_python: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v2 | ||
- uses: actions/setup-python@v2 | ||
- run: pip install tox | ||
- run: tox -e lint | ||
- uses: actions/checkout@v4 | ||
- uses: actions/setup-python@v5 | ||
with: {python-version: 3.x} | ||
- run: pip install --upgrade pip setuptools wheel | ||
cclauss marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- run: pip install black codespell mypy pytest six | ||
- run: black --check . || true | ||
- run: codespell || true # --ignore-words-list="" --skip="" | ||
- run: pip install -e . | ||
- run: mypy --ignore-missing-imports . || true | ||
- run: mv setup.cfg setup.cfg.disabled | ||
- run: pytest --ignore=tests/test_client.py --ignore=tests/test_websocket_integration.py | ||
- run: pytest tests/test_websocket_integration.py || true # Todo: Fix these failing tests | ||
- run: pytest tests/test_client.py || true # Todo: Fix these failing tests |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
# https://docs.astral.sh/ruff | ||
name: ruff | ||
on: | ||
push: | ||
branches: [master] | ||
pull_request: | ||
jobs: | ||
ruff: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v4 | ||
- run: pip install --user ruff | ||
- run: ruff --output-format=github . |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,22 +1,20 @@ | ||
name: tox | ||
on: [push, pull_request] | ||
on: | ||
push: | ||
branches: [master] | ||
pull_request: | ||
jobs: | ||
tox: | ||
strategy: | ||
fail-fast: false | ||
max-parallel: 4 | ||
max-parallel: 5 | ||
matrix: | ||
python: [3.7, 3.8, 3.9] | ||
python: ["3.8", "3.9"] # TODO: Add these... , "3.10", "3.11", "3.12"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can add them, but upgrade pytest in requirements.txt to latest version (7.4.3). |
||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v2 | ||
- uses: actions/setup-python@v2 | ||
- uses: actions/checkout@v4 | ||
- uses: actions/setup-python@v5 | ||
with: | ||
python-version: ${{ matrix.python }} | ||
- run: pip install tox | ||
- if: matrix.python == '3.7' | ||
run: TOXENV=py37 tox | ||
- if: matrix.python == '3.8' | ||
run: TOXENV=py38 tox | ||
- if: matrix.python == '3.9' | ||
run: TOXENV=py39 tox | ||
- run: tox -e py |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,9 +2,9 @@ | |
# -*- coding: utf-8 -*- | ||
|
||
# Copyright (c) 2020 Frank Pagliughi <[email protected]> | ||
# All rights reserved. | ||
# | ||
# This program and the accompanying materials are made available | ||
# All rights reserved. | ||
# | ||
# This program and the accompanying materials are made available | ||
# under the terms of the Eclipse Distribution License v1.0 | ||
# which accompanies this distribution. | ||
# | ||
|
@@ -96,7 +96,7 @@ def on_message(mqttc, userdata, msg): | |
args.append(float(s)) | ||
|
||
# Send the request | ||
topic = "requests/math/" + func | ||
topic = "requests/math/" + func | ||
payload = json.dumps(args) | ||
mqttc.publish(topic, payload, qos=1, properties=props) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,9 +2,9 @@ | |
# -*- coding: utf-8 -*- | ||
|
||
# Copyright (c) 2020 Frank Pagliughi <[email protected]> | ||
# All rights reserved. | ||
# | ||
# This program and the accompanying materials are made available | ||
# All rights reserved. | ||
# | ||
# This program and the accompanying materials are made available | ||
# under the terms of the Eclipse Distribution License v1.0 | ||
# which accompanies this distribution. | ||
# | ||
|
@@ -45,7 +45,7 @@ def on_connect(mqttc, userdata, flags, rc, props): | |
print("Subscribing to math requests") | ||
mqttc.subscribe("requests/math/#") | ||
|
||
# Each incoming message should be an RPC request on the | ||
# Each incoming message should be an RPC request on the | ||
# 'requests/math/#' topic. | ||
def on_message(mqttc, userdata, msg): | ||
print(msg.topic + " " + str(msg.payload)) | ||
|
@@ -83,7 +83,7 @@ def on_log(mqttc, obj, level, string): | |
|
||
|
||
# Typically with an RPC service, you want to make sure that you're the only | ||
# client answering requests for specific topics. Using a known client ID | ||
# client answering requests for specific topics. Using a known client ID | ||
# might help. | ||
mqttc = mqtt.Client(client_id="paho_rpc_math_srvr", protocol=mqtt.MQTTv5) | ||
mqttc.on_message = on_message | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
[tool.ruff] | ||
select = [ | ||
"C4", # flake8-comprehensions | ||
"C90", # McCabe cyclomatic complexity | ||
"E", # pycodestyle errors | ||
"F", # Pyflakes | ||
"ISC", # flake8-implicit-str-concat | ||
"PLC", # Pylint Conventions | ||
"PLE", # Pylint Errors | ||
"PLR091", # Pylint Refactor just for max-args, max-branches, etc. | ||
"W", # pycodestyle warnings | ||
# "A", # flake8-builtins | ||
# "ANN", # flake8-annotations | ||
# "ARG", # flake8-unused-arguments | ||
# "B", # flake8-bugbear | ||
# "BLE", # flake8-blind-except | ||
# "COM", # flake8-commas | ||
# "D", # pydocstyle | ||
# "DJ", # flake8-django | ||
# "DTZ", # flake8-datetimez | ||
# "EM", # flake8-errmsg | ||
# "ERA", # eradicate | ||
# "EXE", # flake8-executable | ||
# "FBT", # flake8-boolean-trap | ||
# "G", # flake8-logging-format | ||
# "I", # isort | ||
# "ICN", # flake8-import-conventions | ||
# "INP", # flake8-no-pep420 | ||
# "INT", # flake8-gettext | ||
# "N", # pep8-naming | ||
# "NPY", # NumPy-specific rules | ||
# "PD", # pandas-vet | ||
# "PGH", # pygrep-hooks | ||
# "PIE", # flake8-pie | ||
# "PL", # Pylint | ||
# "PT", # flake8-pytest-style | ||
# "PTH", # flake8-use-pathlib | ||
# "PYI", # flake8-pyi | ||
# "Q", # flake8-quotes | ||
# "RET", # flake8-return | ||
# "RSE", # flake8-raise | ||
# "RUF", # Ruff-specific rules | ||
# "S", # flake8-bandit | ||
# "SIM", # flake8-simplify | ||
# "SLF", # flake8-self | ||
# "T10", # flake8-debugger | ||
# "T20", # flake8-print | ||
# "TCH", # flake8-type-checking | ||
# "TID", # flake8-tidy-imports | ||
# "TRY", # tryceratops | ||
# "UP", # pyupgrade | ||
# "YTT", # flake8-2020 | ||
] | ||
ignore = [ | ||
"E402", | ||
"E703", | ||
"E711", | ||
"E712", | ||
"E721", | ||
"E741", | ||
"F401", | ||
"F811", | ||
"F841", | ||
"PLC1901", | ||
"PLE1205", | ||
] | ||
line-length = 250 | ||
target-version = "py37" | ||
|
||
[tool.ruff.mccabe] | ||
max-complexity = 32 | ||
|
||
[tool.ruff.pylint] | ||
max-args = 15 | ||
max-branches = 36 | ||
max-returns = 22 | ||
max-statements = 130 | ||
|
||
[tool.ruff.per-file-ignores] | ||
"__init__.py" = ["E402"] | ||
"test/*" = ["S101"] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,50 +1,14 @@ | ||
[tox] | ||
envlist = py{37,38,39} | ||
envlist = py{38,39,310,311,312} | ||
|
||
[testenv] | ||
whitelist_externals = echo make | ||
deps = | ||
-rrequirements.txt | ||
flake8 | ||
allowlist_externals = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can drop the allowlist_externals. We only use pytest in commands which is installed by tox (so not external) |
||
echo | ||
make | ||
commands = | ||
# $EXCLUDE is defined above in testenv:py27 as a workaround for Python 2 | ||
# which does not support asyncio and type hints | ||
flake8 . --count --select=E9,F63,F7,F822,F823 --show-source --statistics {env:EXCLUDE:} | ||
python setup.py test | ||
make -C test test | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why drop the execution of those tests ? I don't think they are run by something else. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Those tests should be converted to be regular They're also not covered by any code coverage so long as that's happening. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree, but they are not yet converted, so in meantime it seems better to kept them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Created #768 to track that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Calling pytest can run old-style unit tests out-of-the-box so while converting tests might be useful, it is not obligatory.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PR #768 will re-introduce test lost from the make -C test |
||
# TODO (cclauss) Fix up all these undefined names | ||
flake8 . --count --exit-zero --select=F821 --show-source --statistics | ||
|
||
# On older Python, flake8 version 4 fail with: | ||
# RecursionError: maximum recursion depth exceeded | ||
[testenv:py37] | ||
deps = | ||
-rrequirements.txt | ||
flake8<4 | ||
|
||
# This lint environment should be the same as the one in .github/work | ||
[testenv:lint] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kept the lint environment, this allow to run linter locally. I want to be able to run lints before pushing to github. The idea is the the tox contains the same linter that are currently present in github workflow, so locally or Github could call I've made few change with what was run by Github lint action, mostly instead of |
||
deps = | ||
bandit | ||
black | ||
codespell | ||
flake8 | ||
isort | ||
mypy | ||
pytest | ||
pyupgrade | ||
safety | ||
-e . | ||
commands = | ||
# The "-" in front of command tells tox to ignore errors | ||
bandit --recursive --skip B101,B105,B106,B110,B303,B404,B603,B324 src | ||
- black --check src | ||
- codespell | ||
flake8 src --count --select=E9,F63,F7,F82 --show-source --statistics | ||
flake8 src --count --exit-zero --max-complexity=29 --max-line-length=167 --show-source --statistics | ||
isort --check-only --profile black src | ||
- mypy --ignore-missing-imports src | ||
safety check | ||
# Tests in these two files are fixed in https://github.com/eclipse/paho.mqtt.python/pull/712 | ||
pytest --ignore=tests/test_client.py --ignore=tests/test_websocket_integration.py | ||
|
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.
Kept the run command as install tox & tox -e lint.
This allow to run the same linter locally