-
Notifications
You must be signed in to change notification settings - Fork 417
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
Cache test data on GHA #909
Conversation
|
Should be able to delete https://github.com/nf-core/test-datasets/blob/sarek3/tests/config/test_data.config and use the usual file from modules, which is more future proof, once and if nf-core/modules#2739 is merged |
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! I'm not a CI expert though, so it may be better to wait for a second review
"type": "string", | ||
"default": "https://raw.githubusercontent.com/nf-core/test-datasets/sarek3", | ||
"description": "Base path / URL for data used in the test profiles", | ||
"help_text": "Warning: The `-profile test` samplesheet file itself contains remote paths. Setting this parameter does not alter the contents of that file.", |
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.
Maybe a bit better description on why you might use this option? I know it's mostly for the CI, but still, users will see this.
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.
It's an hidden params with a default value. I think it should be fine
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 snazzy! FWIW I'd have reviewed quicker if there had a been a description ;-)
Good point. I'll pay more attention to that |
PR checklist
nf-core lint
).nextflow run . -profile test,docker --outdir <OUTDIR>
).docs/usage.md
is updated.docs/output.md
is updated.CHANGELOG.md
is updated.README.md
is updated (including new tool citations and authors/contributors).