-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Ecosystem check: Set --no-fix
#15146
Comments
Wouldn't this remove valuable information about fixes that we get from the ecosystem check? Maybe we should just run it both with and without fixes and use the former to populate the +/- violations and the latter for +/- fixes? |
It helps showing how some fixes enable other fixes but I've found the result more confusing than useful. To the point where differences where often ignored because "the ecosystem check is weird, that must be a false positive". If we have specific use cases where we find this useful, we could consider having a second ecosystem check that always runs with |
I agree that having some output about applied fixes would be useful to have in the ecosystem results (especially in order to catch fixes that are unstable in some scenarios, that weren't covered by the tests), but it needs to be deliberate, with its own section in the results table, rather than incidental, so the results can be interpreted correctly. Right now for a new rule with a fix that either always works or works most of the time we will generally see a lower ecosystem impact than the rule actually has, to an unpredictable degree based on the individual project configurations. |
Hey! I'm working on this and will submit a PR shortly. Feel free to assign it to me. |
I think this happens for projects where the
ruff.toml
orpyproject.toml
containsfix = true
. I also got tripped up by this behavior in some of my pull requests. I think it would be a good idea to change the ecosystem script to pass--no-fix
, so this doesn't happen, since the output will always be interpreted incorrectly when fixes have been applied (this can also lead to other confusing things like new E501 flags, because the fix caused a line to get too long, or an unrelated unfixable violation getting flagged, because the fix caused the violation to be slightly offset, so you see a +1 -1 for that violation).Originally posted by @Daverball in #15139 (comment)
The text was updated successfully, but these errors were encountered: