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

Style: aligned black with ruff settings, added pyupgrade #61

Closed
wants to merge 8 commits into from

Conversation

fstagni
Copy link
Contributor

@fstagni fstagni commented Aug 14, 2023

No description provided.

@fstagni fstagni closed this Aug 15, 2023
@fstagni fstagni reopened this Aug 15, 2023
@fstagni fstagni changed the title Style: aligned black with ruff settings, added isort Style: aligned black with ruff settings, added pyupgrade Aug 16, 2023
@fstagni
Copy link
Contributor Author

fstagni commented Aug 16, 2023

The issue that pytest spots is described in fastapi/typer#533. There's a connected PR, but not merged yet.

.pre-commit-config.yaml Outdated Show resolved Hide resolved
@fstagni
Copy link
Contributor Author

fstagni commented Sep 5, 2023

This PR is ready to be merged. The only question I have is if ruff/pyupgrade should be forbidden to run on the auto-generated client code.

@@ -22,8 +22,8 @@
@app.async_command()
async def login(
vo: str,
group: Optional[str] = None,
property: Optional[list[str]] = Option(
group: Optional[str] = None, # noqa: UP007
Copy link
Member

Choose a reason for hiding this comment

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

Shall we do this at the file level with a comment explaining why?

# ruff: noqa: F841

https://beta.ruff.rs/docs/configuration/#error-suppression

@chrisburr
Copy link
Member

The only question I have is if ruff/pyupgrade should be forbidden to run on the auto-generated client code.

I think we definitely shouldn't, we just risk introducing bugs.

@fstagni fstagni mentioned this pull request Sep 5, 2023
@fstagni
Copy link
Contributor Author

fstagni commented Sep 5, 2023

Superseeded by #74

@fstagni fstagni closed this Sep 5, 2023
@chrisburr chrisburr mentioned this pull request Aug 28, 2024
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