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

Release v2.3.0 #666

Merged
merged 250 commits into from
Feb 13, 2025
Merged

Release v2.3.0 #666

merged 250 commits into from
Feb 13, 2025

Conversation

ramprasadn
Copy link
Collaborator

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/raredisease branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Ensure the test suite passes (nextflow run . -profile test_one_sample,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).

@danilodileo danilodileo self-requested a review February 12, 2025 08:23
Copy link
Contributor

@fellen31 fellen31 left a comment

Choose a reason for hiding this comment

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

Nice! Few comments.

conf/modules/align_sentieon.config Outdated Show resolved Hide resolved
conf/modules/prepare_references.config Outdated Show resolved Hide resolved
conf/modules/qc_bam.config Show resolved Hide resolved
docs/output.md Show resolved Hide resolved
ro-crate-metadata.json Show resolved Hide resolved
subworkflows/local/align_sentieon/main.nf Show resolved Hide resolved
subworkflows/local/prepare_references.nf Show resolved Hide resolved
workflows/raredisease.nf Show resolved Hide resolved
Copy link

@danilodileo danilodileo left a comment

Choose a reason for hiding this comment

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

Great job! Overall good, just a couple of comments:

  • on the folder structure in subworkflow/local. it's a bit inconsistent some subworkflows are inside folder, others are not. + I think the structure is subworkflows/local/name/function/main.nf
  • local modules and local subworkflows don't have meta.yml

These are not problematic at all in terms of pipeline functionality! But I am just pointing them out because of nf-core guidelines.

@ramprasadn ramprasadn requested a review from fellen31 February 12, 2025 12:32
@ramprasadn
Copy link
Collaborator Author

Great job! Overall good, just a couple of comments:

* on the folder structure in subworkflow/local. it's a bit inconsistent some subworkflows are inside folder, others are not. + I think the structure is subworkflows/local/name/function/main.nf

* local modules and local subworkflows don't have meta.yml

These are not problematic at all in terms of pipeline functionality! But I am just pointing them out because of nf-core guidelines.

Thanks, @danilodileo. Very valid points. I will try to address them in the next release.

Copy link
Member

Choose a reason for hiding this comment

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

I assume you updated the README since this file was generated.
Can you regenerate that file using nf-core/tools?

Copy link
Member

@maxulysse maxulysse left a comment

Choose a reason for hiding this comment

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

Looking good to me.
I think the RO crate can be re-generated, but to be honest, it's easy to miss as tools does not regenerate it by default.

@ramprasadn ramprasadn merged commit fe9f0ad into master Feb 13, 2025
21 checks passed
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.

9 participants