-
Notifications
You must be signed in to change notification settings - Fork 749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update fastqc to produce multi-version versions.yml #665
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks awesome @grst @mahesh-panchal ! Couple of minor comments.
Will also need to update the linting code and other sections of the |
yes, checking the yaml for validity would be great! Also, it should not contain |
I had a quick look at the commenting feature, and it seems like it's difficult when the PR comes from a fork (which applies to basically all of them). Which of these issues do we need to address now? For the others we could create a separate issue and follow up later. |
I'm not understanding something here. What's the purpose of |
It parses the |
I'm not asking the right question then.. This is a test to be run by what? I'm not familiar with all the mechanics here of testing. Ah, wait this is meant to be run by the pytest workflow thing? And the |
Exactely. This is what is called a custom test in the pytest-workflow documentation.
So far, we don't test for Testing the checksum alone would not be helpful, as the checksum would be automatically generated by the tools command which doesn't help finding out if it actually is a valid |
Perhaps an alternative is to check an exclude list rather than explicitly refer to a file ( if file in params.modules.excludeFileList ). The exclude list could then be a default parameter in the DSL2 template, but left empty in the test workflows. The only complication is then for people who decide to use nf-core modules in their own workflow need to understand this is a thing too. |
Now, let's hope the CI works with the new #tools version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤩
PR checklist
Closes #XXX
<SOFTWARE>.version.txt
file.label
PROFILE=docker pytest --tag <MODULE> --symlink --keep-workflow-wd
PROFILE=singularity pytest --tag <MODULE> --symlink --keep-workflow-wd
PROFILE=conda pytest --tag <MODULE> --symlink --keep-workflow-wd
Exemplary PR to discuss changes of nf-core/tools#1247
to nf-core/modules.
Once this PR is reviewed, we'll need to update all modules (can be automated
mostly).