-
Notifications
You must be signed in to change notification settings - Fork 3
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
[JOSS Review] - Documentation #48
Comments
re openjournals/joss-reviews#5968 Hi @vinisalazar , Thanks for these. Also linking to #51 regarding your third point as they will be dealt with together. Your first point has been fixed in the README.md to make it easier for beginners to follow - namely, by making an empty conda env and then installing dnaapler (I did not get good results trying to do it all in one line, therefore a beginner will have a harder time!). The second point also has been fixed. To clarify, prodigal is not required in dnaapler at all. This is because pyrodigal is a self contained Python package that reimplements and provides python bindings to Prodigal and therefore does not require it as a dependency. pyrodigal will always be installed with a pip install of dnaapler. Regarding your third point, install.md is overhauled after #51 - the correct numbering will be used. George |
This is a quick comment to say that I think there is an unfinished sentence on line 54 of Line 54 in bf86601
I also think that, other than this typo, the new docs/example.md section is excellent. |
Hi @mkerin, Thanks for picking this up - I've made a fix now in the commit references above. George |
LGTM @gbouras13. I will close this issue for now but @mkerin or @gbouras13 please reopen it if you deem it necessary. |
Hi,
attached are some suggestions for the documentation based on my review:
dnaapler/README.md
Line 20 in 7c67b3b
-c bioconda
, like in the command specified in Line 95 of the same file. This should prevent many first-time users from running into a "package not found" error, as it is unlikely they will have read the Beginner Conda Installation section of the docs. You may also link this section here.dnaapler/README.md
Line 106 in 7c67b3b
dnaapler/docs/install.md
Line 46 in 7c67b3b
I will update this issue if I find anything else.
cc openjournals/joss-reviews#5968
The text was updated successfully, but these errors were encountered: