Skip to content

Fix linting for modules.json to add support for only installing subworkflows from a repository#4287

Open
beatrizsavinhas wants to merge 10 commits into
devfrom
fix-modules-json-lint
Open

Fix linting for modules.json to add support for only installing subworkflows from a repository#4287
beatrizsavinhas wants to merge 10 commits into
devfrom
fix-modules-json-lint

Conversation

@beatrizsavinhas
Copy link
Copy Markdown
Contributor

@beatrizsavinhas beatrizsavinhas commented May 19, 2026

Improve linting for modules.json:

  • Add support for repositories from where only subworkflows have been installed since this was currently not allowed.
  • Add explicit linting error message for when there are either no modules or subworkflows installed for a repository.
  • Add explicit linting error messages for repository links that don't start with "http" - this was an existing check that only returned a general error message.

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

@beatrizsavinhas beatrizsavinhas changed the base branch from main to dev May 19, 2026 08:47
@codecov
Copy link
Copy Markdown

codecov Bot commented May 19, 2026

Codecov Report

❌ Patch coverage is 62.50000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.35%. Comparing base (4c78254) to head (2c305f9).

Files with missing lines Patch % Lines
nf_core/pipelines/lint/modules_json.py 62.50% 6 Missing ⚠️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nf-core nf-core deleted a comment from github-actions Bot May 19, 2026
Comment on lines +28 to +29
if not repo.startswith("http"):
failed.append(f'Repository link for {repo} doesn\'t start with "http" in `modules.json`.')
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is arguably out of scope for the PR however I thought it simplified the logic.

@beatrizsavinhas beatrizsavinhas changed the title Update modules_json.py Improve linting for modules.json May 19, 2026
@beatrizsavinhas beatrizsavinhas marked this pull request as ready for review May 19, 2026 10:16
@beatrizsavinhas beatrizsavinhas changed the title Improve linting for modules.json Fix and improve linting for modules.json May 19, 2026
@beatrizsavinhas beatrizsavinhas changed the title Fix and improve linting for modules.json Fix linting for modules.json to add support for only installing subworkflows from a repository May 19, 2026
@mashehu
Copy link
Copy Markdown
Contributor

mashehu commented May 19, 2026

@nf-core-bot fix linting

@github-actions
Copy link
Copy Markdown
Contributor

⚠️ Cannot proceed with deployment

  • reviewDecision: REVIEW_REQUIRED
  • commitStatus: null

Your pull request is missing required approvals

Copy link
Copy Markdown
Contributor

@mashehu mashehu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution, looks good!
Could you please also add a test case for a modules_json with only subworkflow entries (and one completely empty ones) to tests/modules/test_modules_json.py?

@beatrizsavinhas
Copy link
Copy Markdown
Contributor Author

Thanks for the contribution, looks good! Could you please also add a test case for a modules_json with only subworkflow entries (and one completely empty ones) to tests/modules/test_modules_json.py?

Very fair to request tests! We were trying to quickly patch the bug to move on with development. But I'll spend some extra time adding tests as I do agree it would make the codebase better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Linting for modules.json fails when no modules have been installed from a remote repository

3 participants