Skip to content

Conversation

@alxndrdiaz
Copy link
Member

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/createtaxdb 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).

@alxndrdiaz alxndrdiaz requested a review from jfy133 as a code owner December 8, 2025 17:47
@alxndrdiaz alxndrdiaz self-assigned this Dec 8, 2025
@alxndrdiaz alxndrdiaz added the enhancement Improvement for existing functionality label Dec 8, 2025
@alxndrdiaz alxndrdiaz added this to the 2.1.0: Gracious Goblin milestone Dec 8, 2025
@alxndrdiaz alxndrdiaz linked an issue Dec 8, 2025 that may be closed by this pull request
@alxndrdiaz
Copy link
Member Author

@jfy133 Closed the previous PR, this new PR has the dev branch as target.

@alxndrdiaz
Copy link
Member Author

alxndrdiaz commented Dec 9, 2025

The failed GitHub test could be related to tests/default.nf.test, the following command:

nf-test test tests/default.nf.test --update-snapshot

Fails with the following error message (nf-test 0.9.3, Nextflow 25.10.2, and openjdk version "24.0.2-internal" 2025-07-15):

Error: BUG! exception in phase 'semantic analysis' in source unit './createtaxdb/nf-test.config' Unsupported class file major version 68

A similar error has been reported in the nf-test repo: Failing to run on JDK 24.

@jfy133
Copy link
Member

jfy133 commented Dec 10, 2025

Odd... so you can't run the test locally @alxndrdiaz ?

Also I forogt to mention I put the development checklist on the website, https://nf-co.re/createtaxdb/2.0.0/docs/usage/usage/dev/

Maybe you can confirm you've been through it and checked everything before I dive in (I was hoping end of this week, but I just heard my kid is sick so might not be possible)

@alxndrdiaz
Copy link
Member Author

Odd... so you can't run the test locally @alxndrdiaz ?

Also I forogt to mention I put the development checklist on the website, https://nf-co.re/createtaxdb/2.0.0/docs/usage/usage/dev/

Maybe you can confirm you've been through it and checked everything before I dive in (I was hoping end of this week, but I just heard my kid is sick so might not be possible)

No hurry. I will read the checklist in the meantime. The Nextlow tests work fine locally (nextflow run -resume . -profile test,docker --outdir test_output), but nf-test test tests/default.nf.test --update-snapshot fails with the error described above.

@jfy133
Copy link
Member

jfy133 commented Dec 11, 2025

OK when I get back to this I'll try and replciat ehte erro

Copy link
Member

@jfy133 jfy133 left a comment

Choose a reason for hiding this comment

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

  • I've updated the test snapshot (maybe try running nf-test clean @alxndrdiaz to see if it fixes it
  • I've fixed a small bug in the output samplesheet

In the meantime, I will make the following changes (in addition to the comments below)

  • Update CITATIONS.md
  • Update the methods text
  • Update the CHANGELOG
  • Update the metro map

Copy link
Member

@jfy133 jfy133 left a comment

Choose a reason for hiding this comment

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

Can you look through to make sure you agree with my changes @alxndrdiaz , and if you do I think we are ready - thank you very much for picking this up!

@alxndrdiaz
Copy link
Member Author

@jfy133 LGTM, merging now

@alxndrdiaz alxndrdiaz merged commit ccf67a2 into nf-core:dev Dec 18, 2025
33 of 35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improvement for existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for metacache db

2 participants