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

Update Modules #411

Closed
wants to merge 2 commits into from
Closed

Update Modules #411

wants to merge 2 commits into from

Conversation

eperezme
Copy link

@eperezme eperezme commented Aug 22, 2024

The main thing about this pull request is to update the modules to its latest releases so the pipeline stays alive.

  • Updated all modules to the latest version.
  • Added empty lists to samtools_sort inputs as new version expects an optional input
  • Added empty lists to new optional imputs of multiqc

Any review and improving is wellcomed as this is my first pull request on an external repository

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>).

The test suite does not pass as the PRESEQ module errors even on the unmodified version.

edmundmiller and others added 2 commits January 5, 2024 17:45
Added empty lists to samtools_sort inputs as new version expects an optional input
Added empty lists to new optional imputs of multiqc
@eperezme eperezme requested a review from a team as a code owner August 22, 2024 12:14
Copy link
Collaborator

@edmundmiller edmundmiller 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 taking care of this chore!

The main thing about this pull request is to update the modules to its latest releases so the pipeline stays alive.

Could you clarify what you mean by "so the pipeline stays alive"? Are you having a bug with the modules?

Also, please rebase this PR off the dev branch. It looks like it was branched off master, so that's going to cause a weird history.

I think we fixed the preseq tests failing #382 as well.

Thanks for the contribution!

@sateeshperi
Copy link
Contributor

sateeshperi commented Aug 29, 2024

hey @eperezme thanks for this but, I am about to start a maintenance sweep which will include the module updates. you can close this or rebase off dev so we can review and merge. lmk soon. Thanks again

@edmundmiller edmundmiller changed the title Master Update Modules Aug 30, 2024
@eperezme
Copy link
Author

If @sateeshperi is going to update the modules then there is no need for this pull request. I will close it. Thanks <3

@eperezme eperezme closed this Aug 30, 2024
@sateeshperi
Copy link
Contributor

I see you put in effort to update the test data paths and use the nft-bam plugin. Thank you @eperezme I will make sure to include your contributions in the changelog

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.

3 participants