Skip to content

Conversation

@KazuCocoa
Copy link
Member

@KazuCocoa KazuCocoa commented Oct 9, 2024

#1042

Rules:
https://docs.astral.sh/ruff/rules/

Some customizations such as quote are in .ruff.toml. Max line length comes from current pylint rule.
pyflakes is default one in ruff. I have enabled isort and pylint.

Some diffs between black and default ruff.
https://docs.astral.sh/ruff/formatter/black/

I have applied:

ruff format .
ruff check --fix .

I'm good with this PR format, but if we want to add rules, I'm happy to hear it.

cc @mykola-mokhnach if you have any preferences

]
"**/{test,docs}/*" = [
# https://docs.astral.sh/ruff/rules/magic-value-comparison/
"PLR2004"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to prevent error like below:

test/unit/webdriver/network_test.py:33:45: PLR2004 Magic value used in comparison, consider replacing `2` with a constant variable
   |
31 |             body='{"value": {"wifi": true, "data": false, "airplaneMode": false}}',
32 |         )
33 |         assert driver.network_connection == 2
   |                                             ^ PLR2004
34 | 
35 |     @httpretty.activate
   |

@KazuCocoa KazuCocoa changed the title chore: use ruff (isort, pylint and pyflakes) chore: use ruff (isort, pylint and pyflakes) instead of individual isort, pylint and black libraries Oct 10, 2024
@KazuCocoa KazuCocoa marked this pull request as ready for review October 10, 2024 04:09
Makefile Outdated
.PHONY: mypy
mypy: ## Run mypy
python -m mypy appium test/functional
.PHONY: ruff
Copy link
Contributor

@mykola-mokhnach mykola-mokhnach Oct 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe create two tasks: format + lint for local dev and the task check, which would be called automatically in the precommit hook

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the idea is that we don't want to rely on tool names, but rather intention purposes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense

Copy link
Contributor

@mykola-mokhnach mykola-mokhnach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@KazuCocoa KazuCocoa merged commit 8f2b059 into master Oct 10, 2024
10 checks passed
@KazuCocoa KazuCocoa deleted the apply-ruff branch October 10, 2024 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants