-
Notifications
You must be signed in to change notification settings - Fork 7
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
Added development section to the readme #44
Conversation
I think it's ready to be merged! @ltorres6 |
@MohamedNasser8 Thanks Mohamed, one more iteration for the typos please. I am going to add Lucy and Sirisha to review as well with fresh eyes. |
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.
This looks very good, I agree with Sirisha that perhaps the dev environment setup should just be in the developer guide?
My only other comment is maybe something we can discuss. For the developer who isn't very experienced, the section on poetry and pre-commit hooks could perhaps do with a bit more information - maybe a link to a guide or a simple description about what this does?
Other than that, this looks great.
I think keeping the development environment set in the README for now is important, as it informs anyone coming to the repo on how to start contributing. Once we have a published version, we can update the README accordingly and remove the development setup part. What do you think, @ltorres6 ? |
I think a good compromise is that it should be in both. We should give people a simple overview of how to start developing in the readme for quick start users, and then point to a more detailed version in the docs for users who want/need to know more. This would maintain visibility/exposure of our guidelines from the repo, and allow us to expand in as much detail as we want/need within the docs. |
@MohamedNasser8 Can you add some details on the pre-commit hooks? |
Okay, will do it |
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
In this PR I added the following: