chore: lint scripts/, and put scripts/ under CI lint - #81
Merged
Merged
Conversation
scripts/ had never been linted -- CI ran 'ruff check src tests' -- and had quietly accumulated 26 errors, all in reconcile_volume.py: 22 W293 blank-line-with-whitespace, 1 W291 trailing whitespace, 1 I001 unsorted imports, 2 F541 f-strings with no placeholders. All autofixable, all applied with 'ruff check --fix'. No behaviour changed, and that is verified rather than asserted: the significant-token stream is 575 tokens before and after, and the only differences are the import block regrouped (identical tokens, moved) and two 'f' prefixes dropped from strings containing no placeholders. f"text" and "text" evaluate identically. All five scripts still py_compile. reconcile_volume.py needs Norgate/Windows so it cannot be executed here, which is why the check is token-level rather than behavioural. CI now lints scripts/ too. The selected rules are mostly not style: pyflakes F catches undefined names and unused imports, which are crashes waiting to happen, and scripts/vintage_alert_selftest.py is a diagnostic users are told to run, so a broken one is worse than none. Leaving a source root unlinted also fails open, which is exactly how 26 errors accumulated unnoticed. The trade-off, stated plainly: a future throwaway investigation script now has to pass lint before CI goes green. E501 is already ignored so there is no line-length nagging, and --fix handles nearly everything. To revert, drop 'scripts' from the ruff step; nothing else depends on it. Co-Authored-By: Claude Opus 4.8 <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
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.
scripts/had never been linted — CI runsruff check src tests— and had quietly accumulated 26 errors, all inreconcile_volume.py:All autofixable; applied with
ruff check --fix scripts/.Behaviour is unchanged, and that is verified rather than asserted
reconcile_volume.pyimportscotdata.providers.norgate, which needs Norgate on Windows, so it cannot be executed on this machine. Instead of hand-waving, the check is token-level:fprefixes dropped from strings containing no placeholders.f"text"and"text"evaluate identically.py_compile.git diff -wis 4 lines of import reordering plus the 2 f-prefixes.CI now lints scripts/ too
Decided yes, for three reasons:
Fcatches undefined names and unused imports — crashes waiting to happen, not formatting opinions.scripts/vintage_alert_selftest.pyis user-facing. Users are told to run it to verify their revision alerting works; a broken diagnostic is worse than none.The trade-off, stated plainly: a future throwaway investigation script now has to pass lint before CI goes green.
E501is already ignored so there is no line-length nagging, and--fixhandles nearly everything automatically. To revert, dropscriptsfrom the ruff step — nothing else depends on it.ruff check src tests scriptsclean; 201 tests pass.🤖 Generated with Claude Code