Skip to content
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

Replace local unicycler module with nf-core module + bump version #150

Merged
merged 4 commits into from
Jun 28, 2024

Conversation

FloWuenne
Copy link
Contributor

This PR replaces the local unicycler module with the nf-core/module for unicycler. The nf-core module has nf-tests as well as the newest release version. Updating this module.

Closes #145 .

PR checklist

  • This comment contains a description of changes (with reason).
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • CHANGELOG.md is updated.

@FloWuenne FloWuenne added the enhancement New feature or request label Jun 27, 2024
@FloWuenne FloWuenne changed the title Replaced local unicycler module with nf-core module + bump version Replace local unicycler module with nf-core module + bump version Jun 27, 2024
Copy link

github-actions bot commented Jun 27, 2024

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit e922812

+| ✅ 195 tests passed       |+
#| ❔   4 tests were ignored |#
!| ❗   1 tests had warnings |!

❗ Test warnings:

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 2.14.1
  • Run at 2024-06-27 15:40:23

Copy link
Collaborator

@d4straub d4straub left a comment

Choose a reason for hiding this comment

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

test_dfast.config still contains the unicycler_args = "--no_correct --no_pilon", see here, I assume thats the problem why tests fail?

@FloWuenne
Copy link
Contributor Author

Thanks @d4straub . Wasn't aware of that test file, will fix now!

Copy link
Contributor

@Daniel-VM Daniel-VM left a comment

Choose a reason for hiding this comment

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

Awesome, thanks. I have added a suggestion.

If modules/local/unicycler is no longer used, please remove it. Additionally, the changelog should be updated adding a reference to this PR.

conf/test.config Outdated Show resolved Hide resolved
@FloWuenne
Copy link
Contributor Author

@Daniel-VM Removed empty unicycler params and updated changelog. Let me know if there is anything else or feel free to directly suggest changes and I will incoporate!

@Daniel-VM
Copy link
Contributor

Daniel-VM commented Jun 27, 2024

You can also delete the folder modules/local/unicycler/ since its main.nf and environment.yml files are no longer used after updating to nf-core modules. 👍🏾

@FloWuenne
Copy link
Contributor Author

You can also delete the folder modules/local/unicycler/ since its main.nf and environment.yml files are no longer used after updating to nf-core modules. 👍🏾

Totally forgot to do this before, so sorry!

Copy link
Contributor

@Daniel-VM Daniel-VM left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@FloWuenne FloWuenne merged commit 03f0dc8 into nf-core:dev Jun 28, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants