Open
Conversation
📝 WalkthroughWalkthroughRestructures the migration guide into rule-driven sections with detailed package/ESLint/tooling migration, replaces deprecated Maglev APIs with explicit replacements and verification steps, adds a new paired Eleventy Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 8/10 reviews remaining, refill in 11 minutes and 48 seconds. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@NOTICE.md`:
- Around line 53-55: Add the new technical term "minisearch" to the Vale
allowlist by appending it to the vocabulary file
config/vale/styles/config/vocabularies/Elements/accept.txt (ensure it matches
casing used in NOTICE.md); update the file so Vale recognizes "minisearch" as an
accepted term and commit the change in this PR.
In `@projects/internals/tools/src/context/migration.md`:
- Around line 10-11: Update the migration guidance that currently references
`0.x` public package versions: replace the phrase "`0.x`" and any mentions of
"0.x public packages" with the current stable "1.x" policy and reword the
bullets so they instruct users to use the latest 1.x public package versions
instead of copying old internal versions; also adjust the second bullet that
mentions "The `0.x` public packages..." to state that 1.x public packages
contain the same non-deprecated components as the latest internal releases and
note deprecated/removed APIs require explicit migration (apply same change to
the duplicate lines referenced as also applies to: 123-125).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 92a7d6be-25e2-4270-a0a2-a8bf7bba3682
📒 Files selected for processing (9)
NOTICE.mdprojects/internals/tools/src/context/migration.mdprojects/lint/NOTICE.mdprojects/site/eleventy.config.jsprojects/site/src/_11ty/shortcodes/index.jsprojects/site/src/docs/about/migration.mdprojects/site/src/docs/about/support.mdprojects/site/src/docs/changelog/index.11ty.jsprojects/site/src/docs/patterns/index.md
coryrylan
force-pushed
the
topic-docs-fixup
branch
from
d815318 to
917573a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
NOTICE.md (1)
53-55:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
minisearchto Vale vocabulary allowlist.
minisearchis newly introduced at Line 53 and Line 311 in Markdown content, so it should be added toconfig/vale/styles/config/vocabularies/Elements/accept.txtin the same PR.Suggested patch
--- a/config/vale/styles/config/vocabularies/Elements/accept.txt +++ b/config/vale/styles/config/vocabularies/Elements/accept.txt +minisearchAs per coding guidelines, "
**/*.md: Follow Google developer documentation style guide and write-good rules for prose linting using Vale. Add new technical terms, component names, and abbreviations to config/vale/styles/config/vocabularies/Elements/accept.txt".Also applies to: 311-311
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@NOTICE.md` around lines 53 - 55, The new term "minisearch" appears in Markdown and must be added to the Vale Elements vocabulary allowlist; open the Elements accept.txt vocabulary used by Vale and append the lowercase token minisearch on its own line so Vale stops flagging it, committing the change in the same PR; ensure the exact spelling "minisearch" is used and no surrounding punctuation is added.projects/site/src/docs/about/migration.md (1)
52-58:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate the package version examples to the current release line.
These examples still advertise
0.xpublic packages, which conflicts with the 1.x policy used elsewhere in the docs and will mislead readers following the migration steps. Please verify the current published major before keeping these examples.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@projects/site/src/docs/about/migration.md` around lines 52 - 58, Update the example package versions from the 0.x line to the current published major for each package in the snippet: replace the version strings for "@nvidia-elements/core", "@nvidia-elements/themes", and "@nvidia-elements/styles" with the current 1.x (or whatever the current major) release numbers; verify the actual published major on npm and use that exact major.minor.patch values in the example so the docs align with the 1.x policy used elsewhere.projects/internals/tools/src/context/migration.md (1)
7-12:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate the migration guide to stop advertising
0.xpackages.The Core Rules and dependency example still point readers at
0.x, which conflicts with the stable 1.x guidance elsewhere in the docs and will mislead migrations. Please verify the current published major and update both spots consistently.Also applies to: 123-125
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@projects/internals/tools/src/context/migration.md` around lines 7 - 12, The migration guide still references package versions as `0.x`; update the two occurrences (the sentence "The new packages reset version numbers to `0.x`." and the dependency example referenced in the Core Rules) to the current published major (e.g., `1.x`) and make the text generic so it reads "the current published major (verify npm registry for the latest)" instead of hardcoding `0.x`; also apply the same replacement to the other affected block noted (lines 123-125) so both spots are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@projects/site/src/docs/about/migration.md`:
- Around line 100-112: The after-example uses <nve-tooltip id="my-tooltip"> and
<nve-button popovertarget="my-tooltip"> but omits the required popover attribute
on the target; update the example so the tooltip element (id "my-tooltip")
includes the popover attribute (i.e., add popover to the <nve-tooltip
id="my-tooltip"> element) so the <nve-button popovertarget="my-tooltip"> can
correctly activate the popover.
---
Duplicate comments:
In `@NOTICE.md`:
- Around line 53-55: The new term "minisearch" appears in Markdown and must be
added to the Vale Elements vocabulary allowlist; open the Elements accept.txt
vocabulary used by Vale and append the lowercase token minisearch on its own
line so Vale stops flagging it, committing the change in the same PR; ensure the
exact spelling "minisearch" is used and no surrounding punctuation is added.
In `@projects/internals/tools/src/context/migration.md`:
- Around line 7-12: The migration guide still references package versions as
`0.x`; update the two occurrences (the sentence "The new packages reset version
numbers to `0.x`." and the dependency example referenced in the Core Rules) to
the current published major (e.g., `1.x`) and make the text generic so it reads
"the current published major (verify npm registry for the latest)" instead of
hardcoding `0.x`; also apply the same replacement to the other affected block
noted (lines 123-125) so both spots are consistent.
In `@projects/site/src/docs/about/migration.md`:
- Around line 52-58: Update the example package versions from the 0.x line to
the current published major for each package in the snippet: replace the version
strings for "@nvidia-elements/core", "@nvidia-elements/themes", and
"@nvidia-elements/styles" with the current 1.x (or whatever the current major)
release numbers; verify the actual published major on npm and use that exact
major.minor.patch values in the example so the docs align with the 1.x policy
used elsewhere.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: ad4f71b0-de7b-448d-b1fb-1b0c12bba6b0
📒 Files selected for processing (9)
NOTICE.mdprojects/internals/tools/src/context/migration.mdprojects/lint/NOTICE.mdprojects/site/eleventy.config.jsprojects/site/src/_11ty/shortcodes/index.jsprojects/site/src/docs/about/migration.mdprojects/site/src/docs/about/support.mdprojects/site/src/docs/changelog/index.11ty.jsprojects/site/src/docs/patterns/index.md
coryrylan
force-pushed
the
topic-docs-fixup
branch
from
917573a to
d15534f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
projects/site/src/docs/about/migration.md (1)
52-58:⚠️ Potential issue | 🟠 MajorUpdate migration.md version guidance to reflect current stable 1.x release cycle.
Lines 52-58 recommend
^0.0.xversions and state "a stable 1.0 release follows later," which conflicts with support.md (line 74) documenting that packages are "now in its stable 1.x release cycle." Users following the migration guide will be directed to outdated versions.Suggested doc correction
-+ "@nvidia-elements/core": "^0.0.3", -+ "@nvidia-elements/themes": "^0.0.4", -+ "@nvidia-elements/styles": "^0.0.3" ++ "@nvidia-elements/core": "^1", ++ "@nvidia-elements/themes": "^1", ++ "@nvidia-elements/styles": "^1" ... -The `0.x` versions of the new packages contain the same non-deprecated components as the latest internal releases. Version numbers reset as part of the migration to public npm. A stable 1.0 release follows later. +The stable `1.x` versions of the new packages contain the same non-deprecated components as the latest internal releases. Deprecated and removed APIs still require explicit migration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@projects/site/src/docs/about/migration.md` around lines 52 - 58, Update the migration guidance to reference the current stable 1.x cycle instead of the old 0.x placeholders: change the package version examples for "@nvidia-elements/core", "@nvidia-elements/themes", and "@nvidia-elements/styles" from "^0.0.x" to appropriate "^1.0.x"/"^1.x" pins and replace the sentence "A stable 1.0 release follows later." with wording that matches support.md's statement that these packages are now in a stable 1.x release cycle. Ensure the package list and summary sentence in migration.md are consistent with support.md's current messaging.
♻️ Duplicate comments (2)
projects/site/src/docs/about/migration.md (1)
109-112:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
popoverto the popover target element in the after-example.Line 110 uses a
popovertargetcontrol pattern but the target element is missingpopover, so the example is incomplete.Suggested fix
-<nve-tooltip id="my-tooltip" position="top">hello there</nve-tooltip> +<nve-tooltip id="my-tooltip" popover position="top">hello there</nve-tooltip> <nve-button popovertarget="my-tooltip">tooltip</nve-button>For the HTML Popover API, when an element uses `popovertarget="my-tooltip"`, must the target element with `id="my-tooltip"` include the `popover` attribute?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@projects/site/src/docs/about/migration.md` around lines 109 - 112, The example is missing the required popover attribute on the popover target element: update the nve-tooltip element with id="my-tooltip" to include the popover attribute so it matches the nve-button popovertarget="my-tooltip" control pattern (i.e., ensure the element referenced by popovertarget has popover set).projects/internals/tools/src/context/migration.md (1)
10-11:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace
0.xguidance with stable 1.x migration guidance.These lines still instruct
0.x, which conflicts with current stable policy and can misdirect migrations.Suggested doc correction
-- The new packages reset version numbers to `0.x`. Use the latest available public package versions instead of copying old internal versions. -- The `0.x` public packages contain the same non-deprecated components as the latest internal releases. Deprecated or removed APIs need explicit migration. +- Use the latest stable `1.x` public package versions instead of copying old internal versions. +- The stable `1.x` public packages contain the same non-deprecated components as the latest internal releases. Deprecated or removed APIs still need explicit migration. ... - "@nvidia-elements/core": "0.x", - "@nvidia-elements/themes": "0.x", - "@nvidia-elements/styles": "0.x" + "@nvidia-elements/core": "^1", + "@nvidia-elements/themes": "^1", + "@nvidia-elements/styles": "^1"#!/bin/bash # Verify outdated 0.x guidance in this migration context file. rg -n '0\\.x|\\^1|stable 1\\.x' projects/internals/tools/src/context/migration.mdAlso applies to: 123-125
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@projects/internals/tools/src/context/migration.md` around lines 10 - 11, Update the migration guidance in migration.md to stop referencing "0.x" and instead instruct users to migrate to the stable 1.x line: replace occurrences of "0.x" with "1.x" and change the sentence "Use the latest available public package versions instead of copying old internal versions." to explicitly recommend using stable 1.x public package versions; ensure any examples or notes that mention "^1" or "stable 1.x" are normalized to a single, consistent phrase ("stable 1.x" or "1.x") and remove conflicting guidance about copying old internal versions so the file's recommendation is unambiguous (look for the quoted strings '0.x', '^1', and "stable 1.x" in the file to locate the edits).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@projects/internals/tools/src/context/migration.md`:
- Around line 194-196: The example uses nve-button with
popovertarget="my-tooltip" but the referenced nve-tooltip element is missing the
required popover attribute; update the nve-tooltip element (id="my-tooltip") to
include the popover attribute so the popovertarget reference is valid per the
HTML Popover API (ensure the element with id "my-tooltip" has the popover
attribute and keep the nve-button popovertarget="my-tooltip" as-is).
---
Outside diff comments:
In `@projects/site/src/docs/about/migration.md`:
- Around line 52-58: Update the migration guidance to reference the current
stable 1.x cycle instead of the old 0.x placeholders: change the package version
examples for "@nvidia-elements/core", "@nvidia-elements/themes", and
"@nvidia-elements/styles" from "^0.0.x" to appropriate "^1.0.x"/"^1.x" pins and
replace the sentence "A stable 1.0 release follows later." with wording that
matches support.md's statement that these packages are now in a stable 1.x
release cycle. Ensure the package list and summary sentence in migration.md are
consistent with support.md's current messaging.
---
Duplicate comments:
In `@projects/internals/tools/src/context/migration.md`:
- Around line 10-11: Update the migration guidance in migration.md to stop
referencing "0.x" and instead instruct users to migrate to the stable 1.x line:
replace occurrences of "0.x" with "1.x" and change the sentence "Use the latest
available public package versions instead of copying old internal versions." to
explicitly recommend using stable 1.x public package versions; ensure any
examples or notes that mention "^1" or "stable 1.x" are normalized to a single,
consistent phrase ("stable 1.x" or "1.x") and remove conflicting guidance about
copying old internal versions so the file's recommendation is unambiguous (look
for the quoted strings '0.x', '^1', and "stable 1.x" in the file to locate the
edits).
In `@projects/site/src/docs/about/migration.md`:
- Around line 109-112: The example is missing the required popover attribute on
the popover target element: update the nve-tooltip element with id="my-tooltip"
to include the popover attribute so it matches the nve-button
popovertarget="my-tooltip" control pattern (i.e., ensure the element referenced
by popovertarget has popover set).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 1d63f131-fd45-44f8-90f3-1867cb2e165b
📒 Files selected for processing (7)
projects/internals/tools/src/context/migration.mdprojects/site/eleventy.config.jsprojects/site/src/_11ty/shortcodes/index.jsprojects/site/src/docs/about/migration.mdprojects/site/src/docs/about/support.mdprojects/site/src/docs/changelog/index.11ty.jsprojects/site/src/docs/patterns/index.md
Signed-off-by: Cory Rylan <crylan@nvidia.com>
Signed-off-by: Cory Rylan <crylan@nvidia.com>
coryrylan
force-pushed
the
topic-docs-fixup
branch
from
d15534f to
8703914
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
projects/site/src/docs/about/migration.md (1)
52-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate dependency examples to the stable 1.x policy.
These lines still prescribe
0.x/^0.0.xand “stable 1.0 later,” which conflicts with current 1.x guidance and can drive migrations to stale constraints.Suggested doc correction
-+ "@nvidia-elements/core": "^0.0.3", -+ "@nvidia-elements/themes": "^0.0.4", -+ "@nvidia-elements/styles": "^0.0.3" ++ "@nvidia-elements/core": "^1", ++ "@nvidia-elements/themes": "^1", ++ "@nvidia-elements/styles": "^1" ... -The `0.x` versions of the new packages contain the same non-deprecated components as the latest internal releases. Version numbers reset as part of the migration to public npm. A stable 1.0 release follows later. +Use the latest stable 1.x public package versions. Deprecated or removed APIs still require explicit migration.Also applies to: 58-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@projects/site/src/docs/about/migration.md` around lines 52 - 55, The dependency examples list uses pre-1.0 versions ("@nvidia-elements/core", "@nvidia-elements/themes", "@nvidia-elements/styles" pinned to "^0.0.x"), which conflicts with the current stable 1.x policy; update these example entries to use the stable 1.x range (for example, change the version specifiers for "@nvidia-elements/core", "@nvidia-elements/themes", and "@nvidia-elements/styles" to a 1.x range such as "^1.0.0" or "^1.x") so the docs reflect the current migration guidance.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@projects/site/src/docs/about/migration.md`:
- Around line 52-55: The dependency examples list uses pre-1.0 versions
("@nvidia-elements/core", "@nvidia-elements/themes", "@nvidia-elements/styles"
pinned to "^0.0.x"), which conflicts with the current stable 1.x policy; update
these example entries to use the stable 1.x range (for example, change the
version specifiers for "@nvidia-elements/core", "@nvidia-elements/themes", and
"@nvidia-elements/styles" to a 1.x range such as "^1.0.0" or "^1.x") so the docs
reflect the current migration guidance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 0a8f5ad7-0322-4bc9-8545-fb3ee55bb332
📒 Files selected for processing (7)
projects/internals/tools/src/context/migration.mdprojects/site/eleventy.config.jsprojects/site/src/_11ty/shortcodes/index.jsprojects/site/src/docs/about/migration.mdprojects/site/src/docs/about/support.mdprojects/site/src/docs/changelog/index.11ty.jsprojects/site/src/docs/patterns/index.md
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit
Documentation
New Features
UI Improvements