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

1.0.0 - Ratzupaltuff #135

Merged
merged 534 commits into from
Aug 1, 2023
Merged

1.0.0 - Ratzupaltuff #135

merged 534 commits into from
Aug 1, 2023

Conversation

subwaystation
Copy link
Collaborator

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

Towards a first release of the nf-core/pangenome pipeline. Fingers crossed :)

@maxulysse
Copy link
Member

maxulysse commented Jun 26, 2023

Nope, how could I have done all of these in one commit using the github interface?

If you go in the Files changed tab, you can add a commit to batch and then batch commit several

$query_list \\
--threads $task.cpus \\
$paf_mappings \\
$args > ${prefix}.paf
Copy link
Contributor

Choose a reason for hiding this comment

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

On start, wfmash options do not seem to have defaults

Wfmash Options
wfmash_block_length : null
wfmash_sparse_map : null

shows up as

Wfmash Options
wfmash_block_length N/A
wfmash_sparse_map N/A

in the MultiQC report, under nf-core/pangenome Workflow Summary

Copy link
Collaborator Author

@subwaystation subwaystation Jul 17, 2023

Choose a reason for hiding this comment

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

I was also seeing this and was annoyed.
These parameters can be set, but if they are not set, they will be calculated based on other parameters later. So I am not sure what to do here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because one should never set params.<PARAM> later.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, haven't run into this before. Maybe drop a question on nf-core slack to see if anyone else might have a suggestion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If I don't get an answer by the end of the week, I would resolve this conversation and continue the first release protocol.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@maxulysse @mirpedrol Do you know more here maybe?

main.nf Outdated Show resolved Hide resolved
docs/output.md Outdated Show resolved Hide resolved
docs/output.md Show resolved Hide resolved
@heuermh
Copy link
Contributor

heuermh commented Jul 14, 2023

Ran -profile test,docker locally and did a quick review, will do a closer look this weekend

docs/output.md Outdated Show resolved Hide resolved
docs/output.md Outdated Show resolved Hide resolved
@heuermh
Copy link
Contributor

heuermh commented Jul 14, 2023

I see no blockers, only small doc changes 🎉

Would you like me to suggest changes or pull request against this branch?

@subwaystation
Copy link
Collaborator Author

@heuermh Suggest changes here would be cool!

@subwaystation
Copy link
Collaborator Author

Thanks for the review @heuermh !

README.md Outdated Show resolved Hide resolved
subwaystation and others added 2 commits July 19, 2023 09:50
Co-authored-by: Michael L Heuer <heuermh@acm.org>
@subwaystation
Copy link
Collaborator Author

I think If this is a first release, ask a core team member to activate the Zenodo functionality for this repository, which will be used to generate a DOI. is still missing here @FriederikeHanssen @maxulysse @heuermh.

@heuermh
Copy link
Contributor

heuermh commented Jul 25, 2023

I think If this is a first release, ask a core team member to activate the Zenodo functionality for this repository, which will be used to generate a DOI. is still missing here

Per Slack, we're ready to go!

@subwaystation subwaystation merged commit 9fc8ccd into master Aug 1, 2023
38 checks passed
@heuermh
Copy link
Contributor

heuermh commented Aug 1, 2023

🎉

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.

5 participants