Conversation
Synchronize dev and master branch
* Back to dev 2.3.0dev * Prettier
Important! Template update for nf-core/tools v3.3.2
* Template update for nf-core/tools version 3.4.1 * Apply suggestion from @famosab * Update snaps * rocrate * snap * Apply suggestion from @famosab * Apply suggestion from @famosab * unify the tests --------- Co-authored-by: Famke Bäuerle <45968370+famosab@users.noreply.github.com> Co-authored-by: Famke Bäuerle <famke.baeuerle@gmail.com>
Important! Template update for nf-core/tools v3.5.1
* fix: nf lint to adhere to strict syntax * update all modules and remove versions statements if needed * update sbwf * Changelog and gzi input channel * pre-commit * update local module * update first snap * update snaps * bump nextflow to 25.10.2 * rocrate * Changelog * udpate last modules * udpate last modules * correct snaps * bump nf-schema
* Recover help message * Changelog
* Template update for nf-core/tools version 4.0.0 * Template update for nf-core/tools version 4.0.2 * fix: monochrome logs * recover multiqc title and changelog * Changelog --------- Co-authored-by: Famke Bäuerle <45968370+famosab@users.noreply.github.com> Co-authored-by: Famke Bäuerle <famke.baeuerle@gmail.com>
…/subworkflows}/nf-core/**/tests/` (#139) * Update all modules * remove module tests * remove sbwf tests * gitignore * Update snaps * correct unmap * update modules * synchronize paths * update collate * queue and value * update snaps * update default snap * update changelog * update changelog * prek * changelog * Solve queue issue * Update nextflow_schema.json Co-authored-by: Maxime U Garcia <max.u.garcia@gmail.com> * Update nextflow.config Co-authored-by: Maxime U Garcia <max.u.garcia@gmail.com> --------- Co-authored-by: Maxime U Garcia <max.u.garcia@gmail.com>
|
* Update local sbwf and module to adhere to directory structure * Update snaps * Add meta maps * Add test for local module
* Add tests to local subworkflows * Changelog * try rescuing the value channel * more collect * finalize * avoid overwritng * update snap * Update default test * Apply suggestion from @famosab * simplify test
pinin4fjords
reviewed
May 7, 2026
Member
There was a problem hiding this comment.
AI-assisted review (Claude, on behalf of @pinin4fjords). Posted as comments only - no approve/request-changes.
Reviewed against 891ca2b on the dev branch. The vast majority of the diff is template merges (3.3.1 → 4.0.2) and nf-core modules update output, which I skimmed but did not line-by-line review. Comments below focus on the substantive functional changes (local module rename, fai-handling refactor in #142, local subworkflow restructuring).
The 🔴 high-severity item, the medium .collect()/meta.yaml-content notes, and the low-severity import-cleanup are inline. The body covers only what isn't already on a specific line.
Cross-cutting items not covered inline
- 🟠 Medium —
meta.yaml→meta.ymlrename across all local components.nf-core pipelines lint(4.0.2) flagsmeta_yml_existswarnings on every local subworkflow because they shipmeta.yamlinstead ofmeta.yml— four subworkflows (alignment_to_fastq,pre_conversion_qc,prepare_indices,utils_nfcore_bamtofastq_pipeline) plus the local modulecheckpairedend. Pure rename to fix. - 🟠 Medium — template logo drift. Lint reports
files_unchanged: docs/images/nf-core-bamtofastq_logo_dark.png does not match the template. Auto-fixable withnf-core pipelines lint --dir . --fix files_unchanged.
Upstream-candidacy of the new local components
Since this PR is reshaping the local subworkflows, worth flagging which of the four pieces look like they should live in nf-core/modules or nf-core/subworkflows rather than here:
alignment_to_fastq— the same 4-waySAMTOOLS_VIEW_{MAP,UNMAP}_{MAP,UNMAP}+SAMTOOLS_MERGE+SAMTOOLS_COLLATEFASTQ+CAT_FASTQpattern lives innf-core/sarek/subworkflows/local/bam_convert_samtools/main.nf. The pattern is duplicated across two pipelines today — strong candidate for promotion tonf-core/subworkflowsso both can share, especially since both have to keep the fourwithName: SAMTOOLS_VIEW_{MAP_MAP,UNMAP_UNMAP,...}flag-filter strings in sync via modules.config.pre_conversion_qc— the stats/flagstat/idxstats trio is alreadynf-core/subworkflows/bam_stats_samtoolsupstream. The local sbwf could either reuse that and add the FASTQC branch on top, or upstream a newbam_stats_samtools_fastqcvariant.checkpairedend— generic samtools-only paired-end detection, no equivalent innf-core/modules. Useful to anyone consuming user-supplied BAM/CRAM. Good promotion candidate.prepare_indices— essentially a 2-process wrapper aroundSAMTOOLS_INDEX+SAMTOOLS_FAIDXwith index-detection branching. Marginal upstream value; could be promoted, or left local since most of the logic is the index-flag branching which is pipeline-shaped.
Bundled cleanups (informational)
Nothing release-blocking, grouped here so a single follow-up can clear them:
- The version in
nextflow.config:312is2.3.0devbut PR title isRelease 2.2.1— confirmed offline with the author that thedevsuffix is a leftover from version-bumping; should be2.2.1(and.nf-core.yml/CHANGELOG.mdheading updated to match). - Pre-existing but worth noting next time
assets/schema_input.jsonis touched: line 34 uses"enums"(plural) where JSON Schema is"enum". Silently ignored by nf-schema, so the runtimemeta.filetype != mapped.getExtension()check inutils_nfcore_bamtofastq_pipeline/main.nf:111is doing all the validation work. subworkflow_version: New version availableforsubworkflows/nf-core/utils_nextflow_pipelineandutils_nfschema_plugin.pipeline_todos:toolCitationText/toolBibliographyTextTODO stubs insubworkflows/local/utils_nfcore_bamtofastq_pipeline/main.nfand a TODO in.github/workflows/awsfulltest.yml. Either fill them in or strip the markers.
pinin4fjords
approved these changes
May 7, 2026
Member
pinin4fjords
left a comment
pinin4fjords
left a comment
There was a problem hiding this comment.
Giving you my approval to unblock, I'm sure you will resolve the issues.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR checklist
nf-core pipelines lint).nextflow run . -profile test,docker --outdir <OUTDIR>).nextflow run . -profile debug,test,docker --outdir <OUTDIR>).docs/usage.mdis updated.docs/output.mdis updated.CHANGELOG.mdis updated.README.mdis updated (including new tool citations and authors/contributors).Only missing the version bump - debating between 2.3.0 and 2.2.1!