Add some new terms to the glossary#63
Conversation
There were a bunch of terms that showed up in the word list that weren't defined. Now they are. Plus a couple more.
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-63 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly expands the glossary with many new terms, which is a valuable addition to the documentation. The definitions are generally well-written and informative.
My review focuses on ensuring accuracy, consistency in formatting (especially for internal links), and correctness of the information provided. I've identified a few typos and areas where links could be improved for clarity and correctness.
Summary of Findings
- Typographical Errors: Several entries contain minor typos (e.g., 'issues' instead of 'issued' for CA, 'are using' and 'cetificates' for CSR, 'MOCRE' instead of 'MCORE' for MCORE, 'proucts' instead of 'products' for PID). These have been flagged with suggestions.
- Link Consistency (Trailing Slashes): Multiple new and modified internal links use a trailing slash (e.g.,
../Path/To/File/), which is inconsistent with the more common pattern in this file of omitting the trailing slash for direct links to.mdfiles (e.g.,../Path/To/File). This was noted for lines 29 (commissioning), 36 (DCL), 48 (IM), 58 (NOC), and 89 (VID). A general recommendation is to standardize these. - Link Relevance and Accuracy: The link for NOC (Node Operational Certificate) was updated but points to the Attestation page while its link text is 'Commissioning'. NOCs are more directly covered in the Commissioning process. A correction has been suggested.
- HTTP Link: The link for OpenThread uses HTTP. It should be updated to HTTPS for security best practices.
- Minor Formatting (Not Commented): Some new entries have trailing whitespace at the end of the definition cell (e.g., CI, CLI, CRL, FRP). While Markdown typically handles this, removing it would be cleaner. Per review settings, these low-severity issues were not commented on directly.
Merge Readiness
The pull request adds a lot of useful information to the glossary. However, due to the identified high-severity issues (incorrect term in MCORE definition, misleading link for NOC) and several medium-severity issues (typos, link inconsistencies), I recommend addressing these points before merging. This will ensure the glossary is accurate, professional, and easy to navigate. I am not authorized to approve pull requests, so please ensure further review and approval as per your team's process once changes are made.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-63 |
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-63 |
cecille
dismissed
gemini-code-assist[bot]’s stale review
problems addressed. Gemini man...you need a bit of chill. These were typos.
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-63 |
1 similar comment
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-63 |
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-63 |
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-63 |
Co-authored-by: Jeff Bumgardner <jbumgardner@google.com>
Co-authored-by: Jeff Bumgardner <jbumgardner@google.com>
|
Preview deployed at https://d2doqw88td69eq.cloudfront.net/pr-63 |
3 participants
There were a bunch of terms that showed up in the word list that weren't defined. Now they are. Plus a couple more.