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

Changed path to RDP and SILVA ref files #837

Merged
merged 13 commits into from
Mar 11, 2025

Conversation

chan-98
Copy link

@chan-98 chan-98 commented Mar 5, 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/ampliseq 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).

Fixing path from zenodo.org/record to zenodo.org/records to fix db staging error. Closes #834

@nf-core-bot
Copy link
Member

Warning

Newer version of the nf-core template is available.

Your pipeline is using an old version of the nf-core template: 3.0.2.
Please update your pipeline to the latest version.

For more documentation on how to update your pipeline, please see the nf-core documentation and Synchronisation documentation.

Copy link

@jasmezz jasmezz left a comment

Choose a reason for hiding this comment

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

Please resolve conflicts, assuming all tests succeed.

You updated because the record links do redirect anyway to records, right?

Edit: Ah I see the linked issue now. All clear ;)

@chan-98
Copy link
Author

chan-98 commented Mar 5, 2025

Please resolve conflicts, assuming all tests succeed.

Resolved merge conflicts.

Copy link
Collaborator

@d4straub d4straub left a comment

Choose a reason for hiding this comment

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

Hi there, thanks for the effort, but there seems to be some issue with the PR because Silva 138.2 database entry was removed.

@chan-98 chan-98 requested a review from d4straub March 7, 2025 07:06
Copy link
Collaborator

@d4straub d4straub left a comment

Choose a reason for hiding this comment

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

Looks good to me now.
However, 3 tests are failing that are supposed to not fail, because contents of a file are changing that lists also the link to the chosen database. Could you change that md5sums?
See 1, 2, 3

@chan-98
Copy link
Author

chan-98 commented Mar 10, 2025

@d4straub Ah, thanks! Sorry for not checking this before, I'm quite new to nf-core. I think this should work now?

Edit: I don't understand why the tests are not producing the desired output files, could use some help here.

Copy link
Collaborator

@d4straub d4straub left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

Edit: I don't understand why the tests are not producing the desired output files, could use some help here.

All CI tests are run with nextflow version 24.04.2 and the latest nf version. All tests with nf version 24.04.2 succeed, that is the required part. The latest nf version is however incompatible with the current nf-core template, when updating to th newest template that should be fixed.

Edit: do you want to do the honors and merge the PR?

@chan-98 chan-98 closed this Mar 10, 2025
@chan-98 chan-98 reopened this Mar 10, 2025
@chan-98
Copy link
Author

chan-98 commented Mar 11, 2025

Thanks for the explanation!

Edit: do you want to do the honors and merge the PR?

Ha, I don't think I have the necessary permissions 😄 I see no Merge option

@d4straub d4straub merged commit 68e8278 into nf-core:dev Mar 11, 2025
34 of 62 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.

4 participants