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

Use ruff format #2901

Merged
merged 1 commit into from
Nov 2, 2023
Merged

Conversation

karpetrosyan
Copy link
Member

Ruff introduced a new formatter that can completely replace black, allowing us to use ruff instead of ruff + black.

Ruff formatter, like ruff linter, is extremely fast.

@karpetrosyan karpetrosyan requested a review from a team October 25, 2023 06:17
@Kludex
Copy link
Member

Kludex commented Oct 25, 2023

Announcement: https://astral.sh/blog/the-ruff-formatter

scripts/check Outdated Show resolved Hide resolved
Copy link
Member

@musale musale left a comment

Choose a reason for hiding this comment

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

This looks good to me.

@Kludex Kludex added the do not merge PRs that should not be merged label Oct 26, 2023
pyproject.toml Outdated Show resolved Hide resolved
Copy link
Member

@Kludex Kludex left a comment

Choose a reason for hiding this comment

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

Change to 88 back, please. At least let's discuss first.

Also, what's the time black takes to format httpx?

@Kludex Kludex removed the do not merge PRs that should not be merged label Oct 27, 2023
@karpetrosyan
Copy link
Member Author

Change to 88 back, please. At least let's discuss first.

It was 120 originally, and I just reverted that change 😕 .

Also, what's the time black takes to format httpx?

I don't think it matters; we just have an opportunity to get rid of the extra dependency.

@Kludex
Copy link
Member

Kludex commented Oct 27, 2023

Change to 88 back, please. At least let's discuss first.

It was 120 originally, and I just reverted that change 😕 .

It was 120 on linting... It was 88 on format.

@karpetrosyan
Copy link
Member Author

karpetrosyan commented Oct 27, 2023

It was 120 on linting... It was 88 on format.
Change to 88 back, please. At least let's discuss first.

How do you want me to do it? Both the formatter and the linter have a single line length. I'm not sure what the "change back to 88" even means here.

Setting the formatter line length equal to the linter line length feels more natural to me.

requirements.txt Outdated Show resolved Hide resolved
@karpetrosyan
Copy link
Member Author

Okay, I used the solution that @T-256 suggested, which was to use the max-line-length provided by the pycodestyle settings to keep this pull request as simple as possible.

Later, I'll start a discussion about enforcing limits to the same number.

@Kludex Kludex merged commit 1b7f39e into encode:master Nov 2, 2023
5 checks passed
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.

6 participants