Skip to content
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

Ruff E through P #45

Merged
merged 16 commits into from
Feb 7, 2024
Merged

Ruff E through P #45

merged 16 commits into from
Feb 7, 2024

Conversation

bandophahita
Copy link
Contributor

No description provided.

@bandophahita bandophahita requested a review from perrygoy February 2, 2024 23:57
"EM", # flake8-errmsg
"ERA", # eradicate
"F", # Pyflakes
"FA", # flake8-future-annotations
# "FBT", # flake8-boolean-trap
Copy link
Contributor Author

Choose a reason for hiding this comment

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

purposefully skipping FBT for this PR. Will be addressed in it's own at the end.

Copy link
Member

@perrygoy perrygoy left a comment

Choose a reason for hiding this comment

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

Some small questions for clarity and/or discussion, no notes! Thanks for the work, Marcel!

ruff = {version = "*", optional = true}
ruff = {version = ">=0.2.0", optional = true}
Copy link
Member

Choose a reason for hiding this comment

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

What necessitated this change?

Copy link
Contributor Author

@bandophahita bandophahita Feb 7, 2024

Choose a reason for hiding this comment

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

The new rule changes in the pyproject.toml occurred at that version.
[tool.ruff.lint.***]

Comment on lines -171 to +173
assert annotation == Optional[TargetingError]
assert annotation == "Optional[TargetingError]"
Copy link
Member

Choose a reason for hiding this comment

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

Interesting, i'm assuming this is from the from __future__ import annotations change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. All annotations are strings when using future.

tests/test_resolutions.py Show resolved Hide resolved
Comment on lines +92 to +93
with pytest.raises(ValueError, match="locator tuple length should be 2"):
Target("test").located((By.ID, "foo", "baz")) # type: ignore[arg-type]
Copy link
Member

Choose a reason for hiding this comment

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

I like these changes, too!

@bandophahita bandophahita merged commit c63fab5 into ScreenPyHQ:trunk Feb 7, 2024
11 checks passed
@bandophahita bandophahita deleted the ruff_e-p branch February 7, 2024 19:09
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.

2 participants