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

Add verifybamid2 #243

Merged
merged 7 commits into from
Oct 29, 2024
Merged

Add verifybamid2 #243

merged 7 commits into from
Oct 29, 2024

Conversation

A97paupic
Copy link
Collaborator

@A97paupic A97paupic commented Oct 25, 2024

Description and reviewer info

Add a verifybamid2 process with a if clause to separate between wgs and panel data input. In addition the data used by the verifybamid2 software and container has been added to the config.

Type of change

  • Documentation
  • Patch
  • Minor change
  • Major change

Checklist

  • Self-review of my code
  • Update the CHANGELOG
  • Tag the latest commit (vX.Y.Z format)
  • Log samples used for testing in the Verification_samples_log Excel sheet

Documentation

  • At least one other person has reviewed my changes (not required for trivial changes)

Patch

  • Stub run completes without errors or new warnings
  • At least one other person has reviewed and approved my code (not required for trivial changes)

Major / Minor change

  • Stub run completes without errors or new warnings
  • onco run finishes without any new warnings/errors and the results can
    be loaded into scout
  • wgs single run finishes without any new warnings/errors and the results
    can be loaded into scout
  • wgs trio run finishes without any new warnings/errors and the results
    can be loaded into scout
  • At least one other person has reviewed and approved my code
  • I have made corresponding changes to the documentation (software versions, etc.)

Test/review documentation

Review performed by

  • Alexander
  • Jakob
  • Paul
  • Ryan
  • Viktor

(Add if missing)

Testing performed by

  • Alexander
  • Jakob
  • Paul
  • Ryan
  • Viktor

Copy link
Contributor

@Jakob37 Jakob37 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 💪 Just some minor points and questions.

Don't forget to add the version to the version document.

I would like to take a final quick look before merge when updated.

CHANGELOG.md Outdated Show resolved Hide resolved
configs/nextflow.hopper.config Outdated Show resolved Hide resolved
main.nf Show resolved Hide resolved
main.nf Show resolved Hide resolved
main.nf Show resolved Hide resolved
main.nf Outdated Show resolved Hide resolved
@Jakob37
Copy link
Contributor

Jakob37 commented Oct 28, 2024

Looks nice! But please add the version to the version document before merge (that is part of the same repo, so it is just to change that .md file). I'll approve when I see that it is in.

@A97paupic
Copy link
Collaborator Author

The version document is now updated with verifybamid2!

@Jakob37
Copy link
Contributor

Jakob37 commented Oct 29, 2024

The version document is now updated with verifybamid2!

Great!

I don't see it in the PR here though. Have you forgotten to push the changes?

Copy link
Contributor

@Jakob37 Jakob37 left a comment

Choose a reason for hiding this comment

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

OK, looks nice now!

So, remember before merge:

  • Tag the last commit "v3.11.1"
  • Document testing you have made (I think you ran a panel? and also put it into the "Verification samples log" spreadsheet)

@A97paupic A97paupic merged commit 2f694d5 into master Oct 29, 2024
1 check 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.

2 participants