Meriam van Os review2 fixes #80
Conversation
itrujnara
left a comment
itrujnara
left a comment
There was a problem hiding this comment.
Nice work on the comments, though you've mixed changes from both reviews. Not a huge problem. I have some minor questions.
There was a problem hiding this comment.
I like the idea. Have you made this emoji or verified the license for it?
There was a problem hiding this comment.
Good question, I didn't make the logo, this one has been in use for 5 years for the first pipeline, so didn't think about a license tbh!
See https://github.com/nf-core/coproid/blob/master/assets/img/coproid_logo.png
I can replace it with this one though - the one in _extensions is used for the quarto report.
https://github.com/nf-core/coproid/blob/MeriamOs-review2-fixes/assets/nf-core-coproid_logo_light.png
There was a problem hiding this comment.
I have changed the logo for quarto to be the same as on the README
bin/create_acc2tax.py
Outdated
| # Written by Maxime Borry, released under the MIT license | ||
| # See https://opensource.org/license/mit for details | ||
|
|
||
| """Creates an acc2tax mapping file from a genome FASTA.""" |
There was a problem hiding this comment.
Docstrings should be in imperative mood, i.e., "create a file", not "creates a file"
https://peps.python.org/pep-0257/
| cat <<-END_VERSIONS > versions.yml | ||
| "${task.process}": | ||
| python: \$(python --version | sed 's/Python //g') | ||
| END_VERSIONS |
There was a problem hiding this comment.
Are stubs planned? If not, make an issue for a future release and let me know
There was a problem hiding this comment.
I have added stubs to the local modules now
nf-core/coproid pull request
These changes fix the reviewer comments left by @itrujnara
PR checklist
nextflow run . -profile test,docker).nf-core lint .).docsis updatedCHANGELOG.mdis updatedREADME.mdis updatedLearn more about contributing: CONTRIBUTING.md