Skip to content

Patch release: Fix #449 (wrong contigs from aDNA workflow going to binning)#458

Merged
jfy133 merged 7 commits intonf-core:masterfrom
maxibor:patch
Jun 19, 2023
Merged

Patch release: Fix #449 (wrong contigs from aDNA workflow going to binning)#458
jfy133 merged 7 commits intonf-core:masterfrom
maxibor:patch

Conversation

@maxibor
Copy link
Copy Markdown
Member

@maxibor maxibor commented Jun 14, 2023

Fixing #449

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

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 14, 2023 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 9f86e7d

+| ✅ 155 tests passed       |+
#| ❔   1 tests were ignored |#
!| ❗   2 tests had warnings |!
Details

❗ Test warnings:

  • nextflow_config - Config manifest.version should end in dev: '2.3.1'
  • pipeline_todos - TODO string in methods_description_template.yml: #Update the HTML below to your prefered methods description, e.g. add publication citation for this pipeline

❔ Tests ignored:

  • files_unchanged - File ignored due to lint config: lib/NfcoreTemplate.groovy

✅ Tests passed:

Run details

  • nf-core/tools version 2.7.2
  • Run at 2023-06-19 09:30:27

jfy133
jfy133 reviewed Jun 14, 2023
View reviewed changes
Copy link
Copy Markdown
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.

Conceptually 👍, test is failing though?

Minor requests:

  • please change the new input channel to the standard form, e.g.: contigs_ch -> ch_contigs_for_binrefinement
  • as this is a hotfix patch will also need a patch version bump (to 2.3.1)
  • also needs a changelog update

@skrakau @d4straub this is a very major bug for the aDNA functionality, so we want to do the patch release, which ideally needs another review. I will take responsibility for pulling the changes into dev once the release is made.

jfy133
jfy133 reviewed Jun 15, 2023
View reviewed changes
Comment thread workflows/mag.nf Outdated
d4straub
d4straub reviewed Jun 15, 2023
View reviewed changes
Comment thread workflows/mag.nf Outdated
@jfy133 @d4straub
Update workflows/mag.nf
9b619ad 
Co-authored-by: Daniel Straub <[email protected]>
@jfy133
Copy link
Copy Markdown
Member

jfy133 commented Jun 15, 2023

@maxibor just need version bump, changelog, and ✔️ from @d4straub and we are good to go

@d4straub
Copy link
Copy Markdown
Collaborator

d4straub commented Jun 17, 2023

version bump, changelog

agreed, will approve then

@maxibor maxibor linked an issue Jun 19, 2023 that may be closed by this pull request
Genome binning is performed on uncorrected contigs when selecting the ancient DNA subworkflow #449
Closed
@maxibor
Copy link
Copy Markdown
Member Author

maxibor commented Jun 19, 2023

@d4straub @jfy133 Version and changelog updated

jfy133
jfy133 reviewed Jun 19, 2023
View reviewed changes
Comment thread CHANGELOG.md Outdated
@jfy133 jfy133 self-requested a review June 19, 2023 09:11
d4straub
d4straub approved these changes Jun 19, 2023
View reviewed changes
Copy link
Copy Markdown
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.

LGTM!

@jfy133
Copy link
Copy Markdown
Member

jfy133 commented Jun 19, 2023

Thanks @maxibor ! If we get the ✔️ from Daniel, we can merge and I'll try and do the release today or tomorrow morning (before I have to travel)

@maxibor
Copy link
Copy Markdown
Member Author

maxibor commented Jun 19, 2023

BTW, this PR relies on #451 being merged before since the meta would differ otherwise

@maxibor
Copy link
Copy Markdown
Member Author

maxibor commented Jun 19, 2023

Changes from #451 were integrated in this PR

jfy133
jfy133 approved these changes Jun 19, 2023
View reviewed changes
Copy link
Copy Markdown
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.

Thanks @maxibor ! This indeed makes sense in the patch (as agreed on slack) :)

@jfy133 jfy133 changed the title Fix #449 Fix #449 (wrong contigs from aDNA workflow going to binning) Jun 19, 2023
@jfy133 jfy133 changed the title Fix #449 (wrong contigs from aDNA workflow going to binning) Patch release: Fix #449 (wrong contigs from aDNA workflow going to binning) Jun 19, 2023
@jfy133 jfy133 merged commit 61deef8 into nf-core:master Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jfy133 jfy133 jfy133 approved these changes

@d4straub d4straub d4straub approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Genome binning is performed on uncorrected contigs when selecting the ancient DNA subworkflow

3 participants

@maxibor @jfy133 @d4straub