-
Notifications
You must be signed in to change notification settings - Fork 477
full pass on Style Guide #20307
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
full pass on Style Guide #20307
Conversation
✅ Netlify Preview
To edit notification comments on pull requests, go to your Netlify project configuration. |
Files changed:
|
✅ Deploy Preview for cockroachdb-interactivetutorials-docs canceled.
|
✅ Deploy Preview for cockroachdb-api-docs canceled.
|
ff15278 to
4276c88
Compare
jhlodin
left a comment
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.
Broadly looks good to me, left a couple comments but no major disagreements from me. Will add comments later if other suggestions come to mind.
| - Databases on the cluster | ||
| - Jobs running on the cluster | ||
|
|
||
| ## Inclusivity |
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.
Suggest a quick subsection under here to avoid idioms and terminology that could be confusing when auto-translated (which should be assumed given that we don't have a localization team).
Suggest something like:
### Avoid idioms and other difficult-to-translate terminology
The majority of our non English-speaking readers use a browser translation tool to read our documentation in a preferred language. Avoid using idioms or non industry-standard terms that these tools may translate literally.
- **Avoid:** Configure roles and permissions to draw a line between allowed and disallowed actions.
**Prefer:** Configure roles and permissions to allow and disallow actions.
| Code samples often include placeholder values, to be replaced by values specific to a user's environment. To denote that a value in a code sample is a placeholder value that should be replaced, use curly brackets (`{}`). | ||
|
|
||
| Reference issues and pull requests by their corresponding number, prepended with `#`. | ||
| - **Example:** `SELECT * FROM TABLE {mytable};`. |
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.
Suggest also adding an example from the API docs where these are commonly used in URLs, like
- **Example:** `--url https://cockroachlabs.cloud/api/v1/clusters/{cluster_id}/nodes`
| **Note:** Screenshots are difficult to keep current, and impact accessibility. Use a screenshot only if it is necessary for the user to accomplish a task or understand a feature. For example, in a page on troubleshooting cluster performance, a screenshot of a Contention metric graph in the DB Console might be used to depict a cluster with high contention. The same screenshot would not be necessary in a reference topic about the DB Console user interface. | ||
|
|
||
| Use a warning to express that a piece of information is critical to understand to prevent data loss, security vulnerabilities, or unexpected behavior. | ||
| To optimize accessibility and SEO, a screenshot should be accompanied by text and [alt-text](#write-accessible-documentation) describing its contents. |
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.
Could we include an example here showing the correct syntax to do this? Like:
<img src="{{ 'images/v24.2/topology-patterns/topology_multi-region_hardware.png' | relative_url }}" alt="Multi-region hardware setup" style="max-width:100%" />
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.
Hmm this should be in the separate MarkdownGuide.md, which was largely necessary, but did indeed cause some awkward splits. I'll see what I can do to mitigate this.
rmloveland
left a comment
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!!! thanks for going through this and doing these thoughtful updates!
PS All my comments below are things that can be revisited in other conversations, they're not really about this PR, apologies for the digressions
|
Finally addressed review comments, replaced some previously deleted (from the old style guide and CONTRIBUTING.md) content on page sections, and cross-linked from Style to Markdown guides, if anyone would like to re-review! |
Uh oh!
There was an error while loading. Please reload this page.