wheel icon indicating copy to clipboard operation
wheel copied to clipboard

chore: make sure local ruff runs don't touch vendored

Open henryiii opened this issue 1 year ago • 2 comments

It's useful to run ruff locally sometimes, like to add --unsafe-fixes. This ensures it doesn't format or change something it shouldn't. See https://github.com/pypa/wheel/pull/617.

henryiii avatar May 10 '24 01:05 henryiii

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 71.03%. Comparing base (78b9ea9) to head (74c0d54). Report is 7 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #618   +/-   ##
=======================================
  Coverage   71.03%   71.03%           
=======================================
  Files          13       13           
  Lines        1084     1084           
=======================================
  Hits          770      770           
  Misses        314      314           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar May 10 '24 01:05 codecov[bot]

I suggest you remove this: https://github.com/pypa/wheel/blob/1e00742acc9fb33f6e71460c3844c2b66532af7f/pyproject.toml#L71-L74

It had been forgotten when shifting from black to ruff in https://github.com/pypa/wheel/commit/83b77e591e6f593470f8daddf3bcfd6ca64e81bc.

DimitriPapadopoulos avatar May 10 '24 18:05 DimitriPapadopoulos

We already have this in place which makes this PR redundant.

agronholm avatar Aug 04 '24 12:08 agronholm

Nope. it's not redundant. It's set for pre-commit, but not for standalone ruff runs.

DimitriPapadopoulos avatar Aug 04 '24 13:08 DimitriPapadopoulos

Why not just do pre-commit run -a?

agronholm avatar Aug 04 '24 13:08 agronholm

Because I simply run ruff.

DimitriPapadopoulos avatar Aug 04 '24 13:08 DimitriPapadopoulos

Well, you could just as easily run pre-commit run -a ruff, no need to even install ruff explicitly?

agronholm avatar Aug 04 '24 13:08 agronholm

Yet, I do have ruff installed. When I run ruff, results are not consistent with pre-commit.

DimitriPapadopoulos avatar Aug 04 '24 13:08 DimitriPapadopoulos

How so?

agronholm avatar Aug 04 '24 13:08 agronholm

Are you sure you're running the same version in both cases?

agronholm avatar Aug 04 '24 13:08 agronholm

Of course. It's just that pre-commit skips vendored, while standalone ruff does not.

DimitriPapadopoulos avatar Aug 04 '24 13:08 DimitriPapadopoulos

It’s hard to add flags when running pre-commit, like --unsafe-fixes, so it’s handy to run locally sometimes. Unlike flake8, there’s no variations based on what else is installed, so it’s safe to run it outside pre-commit’s venvs.

Though if all you want to do is run it only with no special flags, I would recommend pre-commit run -a ruff.

henryiii avatar Aug 04 '24 13:08 henryiii

Of course. It's just that pre-commit skips vendored, while standalone ruff does not.

Oh, that's the only thing you meant by results not being consistent?

agronholm avatar Aug 04 '24 13:08 agronholm

It’s hard to add flags when running pre-commit, like --unsafe-fixes, so it’s handy to run locally sometimes. Unlike flake8, there’s no variations based on what else is installed, so it’s safe to run it outside pre-commit’s venvs.

Though if all you want to do is run it only with no special flags, I would recommend pre-commit run -a ruff.

Ah...that's a valid argument. Yeah, I don't see a way to pass flags through pre-commit run. When I needed to use --unsafe-fixes, I just temporarily added that flag to .pre-commit-config.yaml.

agronholm avatar Aug 04 '24 13:08 agronholm

I tend to avoid pre-commit, it doesn't work well outside containers and virtualenvs on older Linux distributions, typically the oldest still maintained Ubuntu LTS.

DimitriPapadopoulos avatar Aug 04 '24 13:08 DimitriPapadopoulos