-
Notifications
You must be signed in to change notification settings - Fork 0
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
Readme #7
Conversation
…management and made changes to the phenotypeomat.toml file to bring it up to date WRT dependencies and naming
…d description of how to interface with the arduino to the README
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some comments and a few small changes but overall this is a great readme! commenting to give @neevor a chance to review, but also ping me when these changes are made. In some places I made suggestions to turn quotes into back ticks. Please feel free to ignore if you don't like that change.
Co-authored-by: Taylor Reiter <[email protected]> Signed-off-by: dgmets <[email protected]>
Co-authored-by: Taylor Reiter <[email protected]> Signed-off-by: dgmets <[email protected]>
Co-authored-by: Taylor Reiter <[email protected]> Signed-off-by: dgmets <[email protected]>
Co-authored-by: Taylor Reiter <[email protected]> Signed-off-by: dgmets <[email protected]>
Co-authored-by: Taylor Reiter <[email protected]> Signed-off-by: dgmets <[email protected]>
Co-authored-by: Taylor Reiter <[email protected]> Signed-off-by: dgmets <[email protected]>
Co-authored-by: Taylor Reiter <[email protected]> Signed-off-by: dgmets <[email protected]>
Co-authored-by: Taylor Reiter <[email protected]> Signed-off-by: dgmets <[email protected]>
… header appear as a header
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm -- three things before merging:
- can you check that the phenotypomat.yml env builds with the command in the readme? I've never seen one with spaces between the package and versions but it might be perfectly valid
- you have a typo in the readme for mediated (meddiated i think). Grammarly was really aggressive when i went to fix it as a suggestion and changed the whole tense of the sentence. totally fine you didn't take the suggest, but the typo is still there!
- there are still two typos in status (written as "statis")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small comments.
|
||
### Compute Specifications | ||
- Arduino Setup: Upload the appropriate .ino sketch to your Arduino device. Ensure that the Arduino is connected to your computer and that the correct port and baud rate are set. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although I'm an awful speller so that might be bad if it looks right to me.
Co-authored-by: Erin <[email protected]> Signed-off-by: dgmets <[email protected]>
I checked that the YAML file creates an env and made the formatting and spelling changes. going to merge. Thank you!! |
I made several changes. Updated the README, updated the pehnotypeomat.toml, and added a YAML file for a conda env.
I apologize in advance for typos... :)
Thank you!!