-
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
feat: update documentation #74
Conversation
-Update readme. -Consolidate information -Rename file names.
-create pr template.
-add contributing file -reorganize information
This is a big PR, next time I will break these down into smaller PRs. |
-formatting
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 is looking really good, thank you @sanchegm! I really like using issue templates and PR templates.
The reason for the doc
directory structure was that we are trying to follow a specific documentation system (the Divio documentation system) where you make a distinction between beginner-friendly tutorials, step-by-step how-to guides for people who have the requisite background knowledge, and long-form explanations (e.g. why some architectural decision was made), hence having those files. I'm not opposed to having individual files for things, but it would be nice if we could continue to use the Divio system since that's what we're using now in gci-vci-aws and in the HCI repo. But if you have strong feelings that it should be done a different way, I'm open to new and better ways of doing things.
Also, it looks like some of the files exceed 80 characters, which I think the auto-formatter should have caught.
change and reformat a few files to comply with Divio documentation guidelines better
CONTRIBUTING.md
Outdated
- If doing something complex or weird, there should be an explanation for it. | ||
|
||
## Conform to best practices | ||
## Best practices |
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.
Nitpick: Can we make the title casing consistent? Could we pick one of the title casing styles in https://titlecaseconverter.com?
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.
Done!
README.md
Outdated
# affils | ||
# ClinGen Affiliation Service | ||
|
||
This service provides a way for ClinGen users to interact (CRUD) with the |
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.
How about:
This service provides a web interface for ClinGen admins to create, view, update, and destroy ClinGen affiliations. ClinGen affiliations are groups of biocurators who work together to curate genes and variants in the gene curation interface (GCI) and the variant curation interface (VCI), respectively. This service also provides a JSON API.
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 great! I have made this change.
|
||
- [Tutorials](./tutorials.md#get-started). Guides to getting set up and other | ||
learning oriented tasks. | ||
- Setting up [environment variables](./how-to.md#environment-variables) |
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.
It might be useful to link to https://12factor.net/config, which provides some background on why you might want to put configuration in environment variables.
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.
Added!
doc/how-to.md
Outdated
|
||
## Contributing | ||
|
||
How to contribute to the Affiliations Serive. |
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.
Typo in "Serive". VS Code has a good spell checker plugin. Also, I don't think "Affiliations Service" needs to be capitalized.
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.
Fixed!
|
||
Please include the issue ticket number and link. This project only accepts pull requests related to open issues. Add `closes #XXXX` in your comment to auto-close the issue that this PR fixes. | ||
|
||
## Checklist |
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 will change once we merge the Django PR.
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.
Should we keep it as is, then update after you merge your PR?
doc/reference.md
Outdated
@@ -0,0 +1,14 @@ | |||
## References |
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.
I wouldn't call this "References". Maybe "Reference Documentation"?
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.
Actually, I don't think this document should exist. I think the plan is to rely on docstrings in the code as reference documentation. If you want, we could use something like Sphinx to generate reference documentation from the docstrings.
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.
Okay! I have removed it!
@@ -1,15 +1,13 @@ | |||
# Explanations | |||
|
|||
Description from |
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.
I like the idea of linking to the Divio description! 👍🏻
Resolving conflicts..
resolve merge conflicts with django
All below issues relate to Issue #21 .
Create Issue and Bug templates. Relates to Issue #75. Closes Add GitHub issue templates #75
Update README.md, consolidate information, rename files. Relates to Issue #76. Closes Describe what the project does in the README #76
Add PR template. Relates to Issue #77. Closes Add PR template #77
Add contributing file. Relates to Issue #78. Closes Add a CONTRIBUTING file #78