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.

Sign up for GitHub

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

Jump to bottom

Update samshee #275

Merged
merged 12 commits into from
Oct 22, 2024
Merged

Update samshee #275

merged 12 commits into from
Oct 22, 2024

Conversation

nschcolnicov
Copy link
Contributor

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/demultiplex 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>).
  • 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).

@nschcolnicov
Copy link
Contributor Author

Created this PR for testing the module, I will update it after the nf-core module has been merged into nf-core/modules

maxulysse
maxulysse reviewed Oct 15, 2024
View reviewed changes
conf/modules.config Outdated
Comment on lines 237 to 239
ext.json_schema_validator = { params.json_schema_validator ? "--schema '${params.json_schema_validator}'" : "" }
ext.name_schema_validator = { params.name_schema_validator ? "--schema '${params.name_schema_validator}'" : "" }
ext.v1 = { params.v1_schema ? "--output-format sectioned" : "" }
Copy link
Member

Choose a reason for hiding this comment

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

I think we can put all that in a ext.args and not ressort to creating non standard ext

Copy link
Contributor Author

@nschcolnicov nschcolnicov Oct 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, I agree, this is still a draft

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for checking in on a draft, just working on a PR on this repo too, and I saw this one appeared and was curious

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha no worries! 😄 .Don't want you to waste your time reviewing something that might change regardless, but any help is always appreciated!

@nschcolnicov nschcolnicov marked this pull request as ready for review October 16, 2024 20:03
@nschcolnicov nschcolnicov requested a review from a team as a code owner October 16, 2024 20:03
@nschcolnicov nschcolnicov requested review from Aratz and removed request for a team October 16, 2024 20:03
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 21, 2024 •
edited
Loading

nf-core pipelines lint overall result: Passed ✅ ⚠️

Posted for pipeline commit a12681f

+| ✅ 202 tests passed       |+
#| ❔   3 tests were ignored |#
!| ❗   8 tests had warnings |!

❗ Test warnings:

  • nextflow_config - Config manifest.version should end in dev: 1.5.2
  • pipeline_todos - TODO string in README.md: Describe the minimum required steps to execute the pipeline, e.g. how to prepare samplesheets.
  • pipeline_todos - TODO string in main.nf: Optionally add in-text citation tools to this list.
  • pipeline_todos - TODO string in main.nf: Optionally add bibliographic entries to this list.
  • pipeline_todos - TODO string in main.nf: Only uncomment below if logic in toolCitationText/toolBibliographyText has been filled!
  • pipeline_todos - TODO string in methods_description_template.yml: #Update the HTML below to your preferred methods description, e.g. add publication citation for this pipeline
  • pipeline_todos - TODO string in awsfulltest.yml: You can customise AWS full pipeline tests as required
  • schema_description - Ungrouped param in schema: strandedness

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 3.0.2
  • Run at 2024-10-21 16:56:39

grst
grst approved these changes Oct 22, 2024
View reviewed changes
Copy link
Member

@grst grst left a comment

Choose a reason for hiding this comment

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

The only downside currently seems that it's not possible to specify multiple samplesheets of any of the json/name/file types.

However, I'm not sure if anyone will ever use that and there are workarounds... So let's not overengineer that for now.

@nschcolnicov nschcolnicov merged commit 4d28759 into nf-core:dev Oct 22, 2024
16 checks passed
@grst grst mentioned this pull request Oct 24, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxulysse maxulysse maxulysse left review comments

@grst grst grst approved these changes

@Aratz Aratz Awaiting requested review from Aratz Aratz is a code owner automatically assigned from nf-core/demultiplex

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nschcolnicov @maxulysse @grst