Conversation
joerick
left a comment
There was a problem hiding this comment.
Looks cool, fewer tools is better. And indeed it is crazy fast. Makes it much more suitable for a pre-commit hook.
cibuildwheel/options.py
Outdated
| image = ( | ||
| pinned_images.get(config_value, config_value) | ||
| if config_value | ||
| else pinned_images["manylinux2014"] | ||
| ) |
There was a problem hiding this comment.
I think my only quibble in this PR is this refactor (and the similar one below). The old version reads clearly, step-by-step, the new one is peculiar, I find it hard to read the intent of the code.
I checked the previous code locally and ruff doesn't seem to disagree with it so maybe the rule has since been ignore'd.
There was a problem hiding this comment.
Maybe old ruff was used. SIM108 was changed to not trigger if the new code will not fit in a single line. astral-sh/ruff#1802
There was a problem hiding this comment.
Yes, old, ancient ruff was used. :P
I'll rerender this later with newer Ruff. Don't want to wipe my usefixtures changes.
| capture_stdout=True, | ||
| ).strip() | ||
| testing_archs: list[Literal["x86_64", "arm64"]] | ||
| testing_archs: list[Literal["x86_64", "arm64"]] # noqa: F821 |
There was a problem hiding this comment.
This one confused me, but it seems like a bug in ruff, I opened a report here - astral-sh/ruff#2105 .
I do wonder if we need F821 at all, given that mypy will always catch this error anyway.
There was a problem hiding this comment.
(Please feel free to keep filing stuff as questions and issues come up. Glad we could resolve this one :))
There was a problem hiding this comment.
It's not resolved, it's just a duplicate of an existing issue (astral-sh/ruff#2005) you helped me open a couple of days ago. :)
There was a problem hiding this comment.
It will go away (Ruff will actually remove it) when this does get fixed, so fine to leave the noqa in here. But we could probably turn off F821 as well, since we are fully typed. It's probably mostly useful if you have code not checked by MyPy.
|
What is need to be done here? |
|
Hmm, maybe it's just on me to rerender? |
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
6857bd5 to
df1f793
Compare
|
Ruff readme consistent with real state :) |
|
Oh sorry, that was sent in a PR, I should've checked first! |
We fixed it on our side 😛 |
Is 10-100x faster (written in Rust), combines flake8 and lots of plugins, pyupgrade, isort, pygrep, yesqa, and some of flake8 (so pre-commit setup is faster - also just a simple binary, I think). And pyproject.toml configured! Also can do auto-fixes for quite a few errors. Ran into a few minor bugs that have already been fixed upstream. Used by Pandas, FastAPI, SciPy, Build, etc. More pre-commit friendly, since we don't have breakage due to plugins not being pinned (because they are all built-in, so there's only the one pre-commit ref).
Suggested in #1399 (comment).