-
Notifications
You must be signed in to change notification settings - Fork 15
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
Adding our nf-test tests to the GitHub Actions CI #66
base: dev
Are you sure you want to change the base?
Conversation
|
Ah, the paths I use to force more CI tests probably don't exist when running in the PR, so I've commented that out. |
Looks like one of the things we're getting hung up on is a missing file for a module test for mesmer:
Looking into how to fix this. |
Easy fix. It just needed a small path change. |
@nf-core-bot fix linting |
Hi @RobJY, |
Looks like everything is passing except this lint error:
Not sure why a file we added is setting off the template "files_unchanged" error. |
@nf-core-bot fix linting |
Hmm, no, bot didn't fix it. |
Ok, that worked, but it seems like more of a workaround than a permanent fix. I just added |
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.
Overall looks good to me! Great job sorting out the linting issues. I just left a couple of questions that are more of a technicality for clarification.
modules/nf-core/basicpy/basicpy.diff
Outdated
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.
Is there a specific reason the module was changed? I have an open PR adding nf-test to basicpy nf-core/modules#6929 if there are some changes to be made to make the module more standardized feel free to add to that PR (it shouldn't be an issue since it's only in MCMICRO currently).
I don't think this is an issue. Especially if the logo is updated down the line, this would be added to |
Co-authored-by: Krešimir Beštak <86408271+kbestak@users.noreply.github.com>
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.
LGTM 🚀
PR checklist
nf-core lint
).nextflow run . -profile test,docker --outdir <OUTDIR>
).nextflow run . -profile debug,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).This is still a work in progress, but I think it's getting close.
To do Items: