-
Notifications
You must be signed in to change notification settings - Fork 50
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
First pass docs edits #162
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lzdanski
requested review from
Lee-W,
pankajastro and
sunank200
as code owners
November 20, 2023 22:40
sunank200
reviewed
Nov 22, 2023
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.
@lzdanski can you fix the pre-commit error as well please before the review?
Lee-W
approved these changes
Nov 28, 2023
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.
Left a few nitpicks. But overall, it looks great!
Co-authored-by: Wei Lee <[email protected]>
pankajastro
approved these changes
Nov 29, 2023
Co-authored-by: Wei Lee <[email protected]>
sunank200
approved these changes
Dec 1, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Did a general sweep for style, grammar, and voice.
I roughly base my suggestions on the Google Developer style guide which is a pretty standard practice. They have a lot of specific usage and grammar rules, but also flexibility to develop our own voice. Happy to talk through any specifics, and there were a couple places I was taking a guess about some missing detail that will need to be checked for technical accuracy.
Sometimes folks ask me to explain the edits I made so here's some consistent conventions I noticed I was adding:
Docs tend to be as active voice as possible and try to avoid using pronouns like "we", "us", and "our." I think this is totally a case by case thing. Dev advocacy has a voice similar to a teacher, so they use "we"/"our", while marketing materials aim for friendly amicability, so they use them as well. Traditional docs typically have a drier tone and sometimes have legal restrictions around the claims they can make regarding recommendations, so it's often avoided entirely or used sparingly. There's some flexibility we have here that traditional product docs might not.
It's risky to emphasize text with
" "
instead of formatting. Sometimes" "
implies to users that the thing being emphasized is supposed to be a string or a literal, when all the intention in writing the sentence was to make emphasize a label. Instead, it's more conventional to use:code/<pre>
for anything that gets entered in a config or executable file, programmatic in general, specific URLs, or example URL's.In docs, if a user needs to take an action in a specific context, for example if they need to change from OpenAI back to their local terminal, it's best to use a sentence structure where we have [Describe context], [describe action] [describe outcome]. Or [Describe outcome], [Describe context], [describe action]. Otherwise, you risk someone executing things in the wrong environment or looking in the wrong place for something.
Docs use headers, bullets, and indentations in a very prescriptive way. There's a few places where everything was aligned at the same bullet/ ordered list level, instead of indenting or showing parent/child relationship in some way. However I didn't have time to get my local build working, just had a chance to do the edits. We need to check my changes in a preview first to make sure the formatting is rendering as expected.
Remaining review items for follow-up PR: