Conversation
shed_lint already routes tools through build_tool_lint_args, so the linters were reachable - only the click options were missing, leaving `shed_lint --tools` unable to run checks `lint` has had for years. Extract both options into options.py factories next to the existing lint_biocontainers_option so the two commands share one definition. Fixes galaxyproject#667 Co-Authored-By: Claude Opus 5 (1M context) <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.
Adds
--doiand--conda_requirementstoplanemo shed_lint, closing an eight-year-old gapbetween it and
planemo lint.The gap
peterjc reported in #667 that
shed_lintwas missing optionslinthas, so checking a repositorymeant running both commands.
--biocontainerwas added independently in 2023 (2c1bbff), withoutreference to the issue; the other two were still missing on master:
lintshed_lint(before)--urls--biocontainer--doi--conda_requirementsplanemo shed_lint --tools --doi .failed withError: No such option '--doi'.Why it was only ever a plumbing gap
The 2017 diagnosis on the issue was that only the click options were missing. That still held:
shed_lint.lint_repositoryalready callsbuild_tool_lint_args(ctx, **kwds)(shed_lint.py:72)and forwards
extra_modulesintolint_tool_source_with(shed_lint.py:143) - the same pathlintuses._lint_extra_moduleskeys offkwds["doi"]andkwds["conda_requirements"](
tool_lint.py:60-73).Since the click endpoint never put those keys in
kwds, the linters silently never loaded. Nodownstream change was needed.
Change
Rather than a third copy of each
@click.optionblock, both are extracted intooptions.pyfactories beside the existing
lint_biocontainers_option(), and used from both commands.docs/commands/lint.rstregenerates byte-identical, confirminglint's interface is untouched.Tests
Two tests in
tests/test_shed_lint.py, both reusing existing fixtures (single_tool_required_files,tests/data/tools/bwa_without_requirements.xml) - no new test data.test_tool_linting_doi- asserts the DOI diagnostic is absent without the flag and present withit. Offline and deterministic: doi.org and the Tool Shed repositories API are stubbed with
responses, astests/test_trs_id.pyalready does. Worth noting doi.org currently answers 403to an unauthenticated
requestsGET, so an unmocked version of this test would be flaky.test_tool_linting_conda_requirements- marked slow, matching the equivalents intest_lint.py.It pins the linter's own message as well as the exit code, since a usage error is also non-zero
and would otherwise pass.
Verified red-to-green: with only the
planemo/changes reverted, both fail (No such option '--doi'); restored, the fulltests/test_shed_lint.pypasses (9 tests), as doestests/test_lint.py(8 passed, 5 skipped). flake8, black and mypy clean on the changed files.Note
--urlsis still defined inline and identically in both commands. Folding it into a factory toowould be a one-line change each, but it is outside this issue - happy to include it if preferred.
Fixes #667
🤖 Generated with Claude Code