Run ruff ci but ignore errors#296
Conversation
|
https://github.com/steinmn/zaptec/actions/runs/17136448312/job/48613481883 demonstrates the ruff check failing due to formatting errors |
|
LGTM. It's a little bit annoying that GH only presents 10 warnings & errors in GHA runs. But that's out of our hands, so I think this is ok. It does require some extra steps to check the result, so for that it might be a little hidden. Is there a way to enforce that there should not be any lint errors in the touched line of the PR? |
Not sure if there is any simple way to do that. You could of course parse the results somehow, but that seems like a lot of effort that I'd rather put towards just solving as many errors as possible and get to the point where we can remove the exit-zero workaround. (I have some follow-up-branches that remove quite a few errors) |
|
Google AI propose: - name: Run Ruff on changed files
run: |
CHANGED_FILES=$(git diff --name-only origin/${{ github.base_ref }}...HEAD -- '*.py' | tr '\n' ' ')
if [ -n "$CHANGED_FILES" ]; then
ruff check $CHANGED_FILES
else
echo "No Python files changed in this PR, skipping Ruff check."
fiNote: While this approach can reduce the scope of the check, it's important to remember that linting errors can sometimes be non-local. A change in one part of a file might introduce an error in an unchanged section, which this method might not catch. For comprehensive linting, running Ruff on the entire codebase remains the most robust approach. Some other finds:
If it turns out to be very troublesome to enforce successful lint on PRs, I think it should be a priority to fix all the current issues. We want to turn on the lint failure as soon as possible. |
|
The AI proposal only really works as long as we don't touch any of the files that still have ruff errors without fixing all the ruff errors in the touched file. I'd rather go with just prioritizing fixing ruff errors. As mentioned, I have been making some progress there, but it's easiest to see the effect if we merge this and I can rebase my branches onto the new master and make PRss from that. |
|
I forgot the conclusion of my post, and I agree: It's not that straight forward to implement, so I'm also keen on just fixing the lint errors than making a lot of machinery to circumvent it. I'll merge this PR. |
Adds ruff validation to make sure we don't regress on formatting and showing our progress towards #258
Thanks to
--exit-zerothe tests still passes despite us still having 296 errors.