Skip to content

Conversation

@erikrikarddaniel
Copy link
Member

@erikrikarddaniel erikrikarddaniel commented Aug 13, 2025

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the pipeline conventions in the contribution docs
  • If necessary, also make a PR on the nf-core/mag branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core pipelines lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Check for unexpected warnings in debug mode (nextflow run . -profile debug,test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

This updates the bbmap/bbnorm module to the current nf-core version plus updates it to give the process only 0.8 of the memory assigned to the task.

Copy link
Contributor

@prototaxites prototaxites left a comment

Choose a reason for hiding this comment

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

LGTM!

You were talking about adding some kind of tunable ext option to this and similar modules - is it worth making the 0.8 tunable here with a parameter, even if that proposal isn't supported for all of nf-core?

@erikrikarddaniel erikrikarddaniel merged commit a079eaa into nf-core:dev Aug 13, 2025
12 checks passed
@erikrikarddaniel
Copy link
Member Author

Sorry, I didn't see your comment until I had merged. I decided not to make this tunable both because I didn't want to go through the bureaucracy of updating the nf-core module but also as it actually felt a bit overengineered for myself.

@erikrikarddaniel erikrikarddaniel deleted the reduce-bbnorm-memory branch August 13, 2025 08:31
@jfy133
Copy link
Member

jfy133 commented Aug 13, 2025

A bit late but @erikrikarddaniel could we have a follow up PR with a change log update please? otherwise is fine!

@erikrikarddaniel
Copy link
Member Author

It's in the change log, under Fixed, or am I mistaken?

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