ci: add a diff-aware pylint check - #98
Merged
Merged
Conversation
VLEAPP had no lint workflow, so nothing verified a pull request beyond whatever the author ran locally. The PR merged on 2026-07-25 went in showing "no checks reported". Ports the check iLEAPP and ALEAPP run. It compares the merge base against the change and fails only on warnings the change introduces, rather than on the absolute count. That distinction matters here: vleapp.py and vleappGUI.py carry 13 and 30 pre-existing warnings, mostly wildcard imports that are how those modules are put together, while the artifact modules are clean. A check that failed on the total would pass artifact-only pull requests and block every core-file one for reasons unrelated to the change, which is the state iLEAPP was in until it moved to this script. The blackboxprotobuf guard step from the iLEAPP workflow is left out, as VLEAPP neither vendors nor imports it. Verified locally in this repo, both directions: linting vleapp.py unchanged against origin/main reports 13 before and after and passes, and appending an f-string with nothing to interpolate makes it report the new warning by name and exit 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
VLEAPP has no lint workflow, so nothing verifies a pull request beyond whatever the author ran locally. The PR merged on 2026-07-25 went in showing "no checks reported".
This ports the check iLEAPP and ALEAPP already run.
Why diff-aware rather than a plain pylint run
The obvious workflow —
pylint <changed files> --disable=C,R— is unusable here. Measured on this repo:vleapp.pyvleappGUI.pyMost of that is wildcard imports, which are how those modules are put together rather than something to fix. A check that failed on the absolute count would pass every artifact-only pull request and fail every core-file one for reasons unrelated to the change. That is precisely the state iLEAPP was in: PRs #1280, #1661 and #1719 all sat red, the last of them while changing nothing but one file it happened to touch.
admin/scripts/lint_changed.pylints the same file set twice — once at the merge base, once at the change — and fails only when a(file, warning)pair appears more often than before. New code is held to the full standard; existing debt neither blocks a pull request nor gets silently widened.Two details in the script matter for a stable comparison, both documented in its docstring: pylint infers across the analysed set, so both runs lint the same path list, and pylint caches between runs, so both pass
--persistent=no.Verified locally, both directions
vleapp.pyunchanged againstorigin/main→ 13 before, 13 after, PASSvleapp.py: f-string-without-interpolation (+1)Difference from the iLEAPP workflow
The blackboxprotobuf guard step is left out — VLEAPP neither vendors nor imports it.
🤖 Generated with Claude Code