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

Perform auto-fix before build the files list #2633

Closed
wants to merge 2 commits into from

Conversation

un-pogaz
Copy link
Contributor

Perform auto-fix (safely) before build the hash for the files list.

@kovidgoyal
Copy link
Owner

Why? The purpose of the check command is to point out files where there
are lint issues allowing the user to fix them in an editor. --fix just
is a convenience to avoid running the editor for files where all the
problems are autofixable.

If you need to autofix files in bulk you can just run

ruff check --fix

there is no real need to have a way to run it via the check command.

@kovidgoyal kovidgoyal closed this Jan 26, 2025
@un-pogaz
Copy link
Contributor Author

un-pogaz commented Jan 26, 2025

Arf, sorry, I should have been clearer.

It's a point of detail because of the current behavior of setup.py check:
The check run Ruff on each file indivilualy if their hash is different from the one in the cache. But if the auto fix is actived and the file only contain autofixable errors, the command ruff check --fix edit the file silently and the hash will be desync from the one that will be stored in the cache, and so the file will be re-inspect during the next checking even it was never edited betwen.
By performing a bulk auto-fix before run_check_files(), the cache of the files builded will contain the hash of the auto-fixed version, and so no useless duplicate check later.

And this edit don't interup the normal file-by-file checking, because the variante command ruff check --fix-only used here only fix what Ruff can auto-fix but don't raise others errors if it can't, and the more several errors will be raise normaly during the file-by-file checking (and the reading of the output is only informative of the numbers of errors auto-fixed). It seamless the same behavior than curently.

Maybe remove the returncode check after the bulk auto-fix if you want to be sure (even if a never see ruff check --fix-only return anything other than 0)

@un-pogaz
Copy link
Contributor Author

I'm made a new commit (that don't show): run_auto_fix() is now only informative and will don't interup the checking if the bulk auto-fix fail.

@kovidgoyal
Copy link
Owner

Ah, I see what you are saying, but the common use case is to have just a few files requiring fixes. So re-checking those few files on next run is less expensive than checking every file on every fix run in advance.

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