Internal documentation#442
Merged
Merged
Conversation
adespawn
force-pushed
the
docs
branch
2 times, most recently
from
452f2ac to
a44960f
Compare
wprzytula
requested changes
May 27, 2026
| When using napi-classes or node-built in type representations (integers, bigInt, arrays, results*, strings, buffers), in regular sync function, | ||
| Napi-rs will generate type annotations correctly in the `index.d.ts` file upon compilation of the code. | ||
|
|
||
| \* result type is not "visible" across, since napi-rs does not annotate what the function can throw, but is properly handled as a passthrough type. |
Contributor
There was a problem hiding this comment.
Missing code example + Result<T> example. Let's say that T is that passthrough type.
|
Warning Review limit reached
More reviews will be available in 33 minutes and 27 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe PR updates documentation across several files. The Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
adespawn
force-pushed
the
docs
branch
3 times, most recently
from
6b4ea18 to
dc20642
Compare
coderabbitai
Bot
added
the
documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/source/internal/on-types.md (1)
1-27:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix critical typos throughout the document.
This file has multiple typos that impact clarity and professionalism:
- Line 3: Duplicate word "ignore ignore" → should be "You can ignore the call this library with arbitrary values"
- Line 7: "Withing" → "Within"
- Line 14: "builtin" → "built-in"
- Line 16: "above mentioned" → "above-mentioned" (hyphenated)
- Line 26: "sens" → "sense"
✏️ Suggested fixes
-JavaScript does allow you to ignore ignore the call this library with arbitrary values as arguments. +JavaScript does allow you to call this library with arbitrary values as arguments. -Withing JavaScript Objects we allow for some duck-typing - this applies to all of the config options and named parameters. +Within JavaScript Objects we allow for some duck-typing - this applies to all config options and named parameters. When it comes to built-in types, we may perform some explicit checks to ensure you have provided expected builtin type. +When it comes to built-in types, we may perform some explicit checks to ensure you have provided expected built-in type. -In general, lacks specific policy on allowing duck-typing: in many places like configuration it's allowed, but there are exceptions -as above mentioned policies. +In general, there lacks a specific policy on allowing duck-typing: in many places like configuration it's allowed, but there are exceptions +as above-mentioned policies. -or where possible allow for duck-typing (ie. don't explicitly disallow it where it makes sens) +or where possible allow for duck-typing (i.e. don't explicitly disallow it where it makes sense)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/internal/on-types.md` around lines 1 - 27, Fix five typos throughout the document to improve clarity and professionalism: In the opening paragraph starting with "JavaScript does allow you", remove the duplicate word "ignore" that appears twice consecutively. Change "Withing JavaScript Objects" to "Within JavaScript Objects". In the paragraph discussing built-in types, change "builtin" to "built-in" with a hyphen. In the same section, change "above mentioned" to "above-mentioned" using a hyphen. Finally, in the closing paragraph about duck-typing, change "sense" at the end of the sentence containing "where it makes sens" to "sense".Source: Linters/SAST tools
docs/source/internal/SUMMARY.md (1)
27-36:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd reference to
release-process.mdin the Sections list.The Sections list should include the new
release-process.mdfor consistency, as it is a key internal documentation file being added alongside the others.📝 Suggested addition
- [Napi](./napi.md) - [Miscellaneous](./miscellaneous.md) - [On types](./on-types.md) +- [Release process](./release-process.md)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/internal/SUMMARY.md` around lines 27 - 36, Add a new bullet point entry for the release-process.md documentation file to the Sections list in the SUMMARY.md file. The new entry should follow the same markdown format as the existing entries (like Errors, ParameterWrapper, Query options overview, Logging, Napi, Miscellaneous, and On types) with a descriptive label linked to the release-process.md file path. Place it appropriately within the list to maintain a logical ordering of the documentation sections.
♻️ Duplicate comments (1)
docs/source/internal/napi.md (1)
106-121:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
#[napi]macro annotation to the code example.Per previous review feedback, the Rust code example is incomplete. The
#[napi]macro annotation should be shown for the async function example.📝 Suggested fix
```rs // Simple version + #[napi] pub async fn query_single_page_encoded( &self, ... ) -> Result<PagingResultWithExecutor> {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/internal/napi.md` around lines 106 - 121, The code example for the simple version of the query_single_page_encoded function is incomplete and missing the #[napi] macro annotation. Add the #[napi] macro attribute above the pub async fn query_single_page_encoded declaration in the simple version example to make it consistent with the full version shown below it and demonstrate the complete, correct usage pattern.
🧹 Nitpick comments (1)
docs/source/internal/napi.md (1)
25-30: 💤 Low valueApply minor grammar and style corrections.
LanguageTool flagged several minor style issues:
- Line 26: Replace "quite easy to understand" (weak intensifier) with a stronger phrasing, and add hyphen for "easy-to-understand interface"
- Line 92: Add hyphen for "node-built-in type representations"
✏️ Suggested edits
-The napi-rs offers [napi-rs classes](https://napi.rs/docs/concepts/class). They offer a quite easy to understand interface. +The napi-rs offers [napi-rs classes](https://napi.rs/docs/concepts/class). They offer a straightforward, easy-to-understand interface.And at line 92:
-When using napi-classes or node-built in type representations (integers, bigInt, arrays, `Result`*, strings, buffers), in regular sync function, +When using napi-classes or node-built-in type representations (integers, bigInt, arrays, `Result`*, strings, buffers), in regular sync function,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/internal/napi.md` around lines 25 - 30, Apply the following style corrections to the documentation: Replace the weak intensifier "quite easy to understand interface" in the first section with stronger phrasing using hyphenated form "easy-to-understand interface" to improve readability and meet style guidelines. Additionally, locate line 92 and add a hyphen to "node-built-in" in the phrase about type representations so it reads "node-built-in type representations" for consistent hyphenation of compound adjectives.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/source/index.rst`:
- Around line 12-13: In the index.rst file, locate the sentence "This driver is
considered production ready" and change "production ready" to "production-ready"
by adding a hyphen between the two words to properly format the compound
adjective.
In `@docs/source/internal/release-process.md`:
- Around line 3-4: Fix the typo on line 3 where "Whe" should be "We" at the
beginning of the sentence. Additionally, replace the vague reference to "n=2
platforms" with specific platform names that are actually supported (such as
"x86_64 Linux and aarch64 Linux") to make the documentation clearer and more
helpful for readers who need to understand which platforms are currently
supported.
In `@docs/source/statements/parametrized-queries.md`:
- Around line 21-22: In the named parameters section of the
parametrized-queries.md documentation, remove the article "a" from the phrase
"use a JavaScript object properties" to correct the grammar. The corrected
phrase should read "use JavaScript object properties to define the parameters"
where the text describes how to declare named markers in queries using
JavaScript object properties.
In `@README.md`:
- Line 11: In the README.md file, locate the phrase "production ready" in the
opening statement about the driver. Change it to "production-ready" by adding a
hyphen between "production" and "ready" to form the correct compound adjective.
This is a simple text replacement where the compound adjective modifying the
driver's status needs proper hyphenation formatting.
---
Outside diff comments:
In `@docs/source/internal/on-types.md`:
- Around line 1-27: Fix five typos throughout the document to improve clarity
and professionalism: In the opening paragraph starting with "JavaScript does
allow you", remove the duplicate word "ignore" that appears twice consecutively.
Change "Withing JavaScript Objects" to "Within JavaScript Objects". In the
paragraph discussing built-in types, change "builtin" to "built-in" with a
hyphen. In the same section, change "above mentioned" to "above-mentioned" using
a hyphen. Finally, in the closing paragraph about duck-typing, change "sense" at
the end of the sentence containing "where it makes sens" to "sense".
In `@docs/source/internal/SUMMARY.md`:
- Around line 27-36: Add a new bullet point entry for the release-process.md
documentation file to the Sections list in the SUMMARY.md file. The new entry
should follow the same markdown format as the existing entries (like Errors,
ParameterWrapper, Query options overview, Logging, Napi, Miscellaneous, and On
types) with a descriptive label linked to the release-process.md file path.
Place it appropriately within the list to maintain a logical ordering of the
documentation sections.
---
Duplicate comments:
In `@docs/source/internal/napi.md`:
- Around line 106-121: The code example for the simple version of the
query_single_page_encoded function is incomplete and missing the #[napi] macro
annotation. Add the #[napi] macro attribute above the pub async fn
query_single_page_encoded declaration in the simple version example to make it
consistent with the full version shown below it and demonstrate the complete,
correct usage pattern.
---
Nitpick comments:
In `@docs/source/internal/napi.md`:
- Around line 25-30: Apply the following style corrections to the documentation:
Replace the weak intensifier "quite easy to understand interface" in the first
section with stronger phrasing using hyphenated form "easy-to-understand
interface" to improve readability and meet style guidelines. Additionally,
locate line 92 and add a hyphen to "node-built-in" in the phrase about type
representations so it reads "node-built-in type representations" for consistent
hyphenation of compound adjectives.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: aab18b27-890c-4aa6-a21c-5c140d3d3b5d
📒 Files selected for processing (9)
README.mddocs/source/index.rstdocs/source/internal/SUMMARY.mddocs/source/internal/miscellaneous.mddocs/source/internal/napi.mddocs/source/internal/on-types.mddocs/source/internal/release-process.mddocs/source/statements/index.mddocs/source/statements/parametrized-queries.md
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the driver’s public-facing and internal documentation to reflect current project status (including “production ready”), expands the statements documentation, and adds/extends internal developer notes for maintainers.
Changes:
- Update README + Sphinx landing page to describe the driver as production ready and refresh the feature/roadmap wording.
- Add a new “Parameterized queries” statements page and include it in the statements toctree.
- Add/expand internal documentation pages (release process, JS/TS typing notes, napi guidance, miscellaneous notes) and link them from the internal summary.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates project status messaging and refreshes the feature list/roadmap wording. |
| docs/source/index.rst | Mirrors the “production ready” + features/roadmap messaging in the Sphinx landing page. |
| docs/source/statements/index.md | Adds the new parameterized-queries page to the statements toctree. |
| docs/source/statements/parametrized-queries.md | New page documenting positional vs named parameter binding. |
| docs/source/internal/SUMMARY.md | Expands internal summary with naming shortcuts and links to new internal pages. |
| docs/source/internal/release-process.md | New internal notes describing packaging/platform constraints and release steps. |
| docs/source/internal/on-types.md | New internal notes on JS/TS typing expectations and duck-typing policy. |
| docs/source/internal/napi.md | New internal guidance on Node-API vs napi-rs and project-specific interop patterns. |
| docs/source/internal/miscellaneous.md | New internal notes on legacy DataStax-specific API handling and other topics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
Extracted public docs changes to #480 |
wprzytula
requested changes
Jun 17, 2026
wprzytula
left a comment
wprzytula
left a comment
Contributor
There was a problem hiding this comment.
I stopped the review. The quality of the new parts is very bad. Please reread it carefully, fix typos, improve clarity.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/source/internal/release-process.md`:
- Around line 6-9: The "Adding a new package" section lacks clarity about what
"trusted publishing" means and why it should be used instead of tokens in CI.
Expand this section to explain that trusted publishing uses OpenID Connect
(OIDC)-based authentication from GitHub Actions to npm for short-lived,
cryptographically secure credentials instead of long-lived tokens. Clarify how
the authentication mechanism works (npm CLI automatically detects GitHub Actions
CI environment, requests an OIDC token, and exchanges it for a short-lived npm
token). Also explain why the initial release must use 0.0.0 or 0.0.1 with
minimal package structure (only package.json and empty index file), and consider
adding a reference link to npm's official trusted publishing documentation for
additional context.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: adec2318-fedd-489d-9bed-c972c546820f
📒 Files selected for processing (7)
MAINTENANCE.mdREADME.mddocs/source/internal/SUMMARY.mddocs/source/internal/miscellaneous.mddocs/source/internal/napi.mddocs/source/internal/on-types.mddocs/source/internal/release-process.md
✅ Files skipped from review due to trivial changes (2)
- docs/source/internal/SUMMARY.md
- README.md
annastuchlik
approved these changes
Jun 18, 2026
annastuchlik
left a comment
annastuchlik
left a comment
Contributor
There was a problem hiding this comment.
I didn't check the internal documentation. The rest is fine.
adespawn
force-pushed
the
docs
branch
2 times, most recently
from
50839b9 to
8408a4a
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/source/internal/miscellaneous.md (1)
42-73:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse professional terminology instead of internet slang in internal documentation.
Line 61 uses "Idk" (internet slang for "I don't know"), which is too informal for internal technical documentation. Replace with standard phrasing like "It is unclear" or "The current usage is unknown."
Additionally, the opening of line 42 ("Well, maybe not exactly personal") uses casual tone that could be tightened for consistency with professional docs standards.
✏️ Suggested fix
- Is someone using those overloads? Idk. Will it be hard for users to remove usages of those overloads? Probably not. + Is someone using those overloads? It is unclear. Will it be hard for users to remove usages of those overloads? Probably not.Also consider:
- Well, maybe not exactly personal, but still. This list is composed of small and big annoyances with the current state of the driver. + This list documents known annoyances and design trade-offs in the current driver implementation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/internal/miscellaneous.md` around lines 42 - 73, Replace informal internet slang and casual phrasing in the "Personal vendettas" section with professional terminology. Specifically, replace "Idk" (which appears in the paragraph about API overloads) with professional phrasing such as "It is unclear" or "The current usage is unknown." Additionally, replace the casual opening phrase "Well, maybe not exactly personal, but still" with more professional and direct wording that maintains clarity while adhering to technical documentation standards.
♻️ Duplicate comments (1)
docs/source/internal/release-process.md (1)
3-4:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSpecify actual platform names instead of abstract notation.
Lines 3-4 use abstract notation ("n=2 platforms", "n+1 npm packages") that requires readers to infer what platforms and package counts are supported. Replace with concrete specifications for clarity.
Based on MAINTENANCE.md CI matrix, specify: "2 platforms: x86_64-unknown-linux-gnu and aarch64-unknown-linux-gnu. 3 npm packages: 1 general package with all JS code, plus 1 per architecture."
✏️ Suggested fix
- Currently we have support for n=2 platforms. This limitation for specific platforms comes from the need to compile and test a given platform in CI. - We have n+1 npm packages. 1 general package with all of the JS code, and additional packages 1 per each architecture. + Currently we support 2 platforms: x86_64-unknown-linux-gnu and aarch64-unknown-linux-gnu. This limitation comes from CI compilation and testing constraints. + We maintain 3 npm packages: 1 general package containing all JS code, plus 1 per supported architecture.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/internal/release-process.md` around lines 3 - 4, Replace the abstract notation in the release-process.md documentation with concrete platform and package specifications. Change "n=2 platforms" to explicitly list the two supported platforms: x86_64-unknown-linux-gnu and aarch64-unknown-linux-gnu. Replace "n+1 npm packages" with the concrete count of 3 npm packages, clarifying that this includes 1 general package with all JS code plus 1 package per architecture. This makes the documentation immediately clear without requiring readers to deduce the specific values.
🧹 Nitpick comments (1)
docs/source/internal/napi.md (1)
37-38: ⚖️ Poor tradeoffExpand on the specific limitations introduced by the custom solutions.
The text states "This is necessary since our solutions introduce some limitations at the benefit of better usability / performance in our specific use cases," but does not enumerate what those limitations are. For readers designing new features, clarity on the constraints is valuable.
For example, do the limitations include: inability to add methods post-creation? read-only fields? lack of inheritance support? type-checking constraints?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/internal/napi.md` around lines 37 - 38, The documentation in the workarounds section mentions that solutions introduce "some limitations" but does not specify what those limitations actually are, making it unclear for readers designing new features. After the statement about solutions introducing limitations, add a dedicated paragraph that explicitly enumerates the specific constraints and limitations that prevent objects from having methods (such as inability to add methods post-creation, read-only field restrictions, lack of inheritance support, type-checking constraints, or other relevant limitations specific to the NAPI implementation). Include concrete examples to clarify each limitation so readers understand the tradeoffs they will face when designing new features.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/source/internal/on-types.md`:
- Line 3: The sentence on line 3 starting with "JavaScript allows you to ignore
calls to this library with arbitrary values as arguments" is grammatically
ambiguous and could be misinterpreted as suggesting you can skip calling the
library. Clarify the intended meaning by rephrasing to explicitly state that the
library can be called with arbitrary argument values, rather than using the word
"ignore" which implies skipping or omitting the calls entirely. Use clearer
language such as "JavaScript allows you to call this library with arbitrary
values as arguments" or similar phrasing that removes the ambiguity.
In `@docs/source/internal/release-process.md`:
- Around line 8-10: Expand the trusted publishing section in the
release-process.md file to provide comprehensive context. Replace the current
brief explanation with a more detailed description that covers: what trusted
publishing is (explain it as OIDC-based authentication from GitHub Actions to
npm registry that replaces long-lived tokens), why tokens and keys should not be
used in CI (clarify that trusted publishing provides cryptographically secure
short-lived credentials, reducing attack surface), how the mechanism works
(describe that npm CLI automatically detects GitHub Actions environment,
requests an OIDC token, and exchanges it for a short-lived npm publishing token
without manual NPM_TOKEN setup), and the initial release exception workflow
(clarify that the first version release requires manual npm CLI authentication
since the registry doesn't yet know about the package, but subsequent releases
use the trusted publishing configured in step 8). Optionally reference the npm
trusted publishing documentation link or expand the explanation inline for
clarity.
- Line 10: The opening sentence on line 10 uses informal first-person language
"What I have done to release a new package is create..." which is inappropriate
for professional documentation. Reword this sentence to use impersonal,
professional style that describes the release process objectively rather than
from a personal perspective. The revised sentence should maintain the same
information about creating an empty index.js and package.json but expressed in a
formal, documentation-appropriate tone.
---
Outside diff comments:
In `@docs/source/internal/miscellaneous.md`:
- Around line 42-73: Replace informal internet slang and casual phrasing in the
"Personal vendettas" section with professional terminology. Specifically,
replace "Idk" (which appears in the paragraph about API overloads) with
professional phrasing such as "It is unclear" or "The current usage is unknown."
Additionally, replace the casual opening phrase "Well, maybe not exactly
personal, but still" with more professional and direct wording that maintains
clarity while adhering to technical documentation standards.
---
Duplicate comments:
In `@docs/source/internal/release-process.md`:
- Around line 3-4: Replace the abstract notation in the release-process.md
documentation with concrete platform and package specifications. Change "n=2
platforms" to explicitly list the two supported platforms:
x86_64-unknown-linux-gnu and aarch64-unknown-linux-gnu. Replace "n+1 npm
packages" with the concrete count of 3 npm packages, clarifying that this
includes 1 general package with all JS code plus 1 package per architecture.
This makes the documentation immediately clear without requiring readers to
deduce the specific values.
---
Nitpick comments:
In `@docs/source/internal/napi.md`:
- Around line 37-38: The documentation in the workarounds section mentions that
solutions introduce "some limitations" but does not specify what those
limitations actually are, making it unclear for readers designing new features.
After the statement about solutions introducing limitations, add a dedicated
paragraph that explicitly enumerates the specific constraints and limitations
that prevent objects from having methods (such as inability to add methods
post-creation, read-only field restrictions, lack of inheritance support,
type-checking constraints, or other relevant limitations specific to the NAPI
implementation). Include concrete examples to clarify each limitation so readers
understand the tradeoffs they will face when designing new features.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8c348a9b-e31c-4731-babe-6382c9e52a54
📒 Files selected for processing (7)
MAINTENANCE.mdREADME.mddocs/source/internal/SUMMARY.mddocs/source/internal/miscellaneous.mddocs/source/internal/napi.mddocs/source/internal/on-types.mddocs/source/internal/release-process.md
✅ Files skipped from review due to trivial changes (2)
- README.md
- docs/source/internal/SUMMARY.md
Comment on lines
+8
to
+10
| To release a new package, follow npm documentation. Once the package is added, add support for [trusted publishing](https://docs.npmjs.com/trusted-publishers). | ||
| DO NOT USE tokens/keys for releasing through CI. You may use those only for the first version release. | ||
| What I have done to release a new package is create an empty index.js and package.json: |
There was a problem hiding this comment.
Expand trusted publishing explanation and clarify initial release workflow.
The current explanation of "trusted publishing" is too brief and lacks critical context. Past reviewers flagged this as cryptic. The section should explain:
- What trusted publishing is: OpenID Connect (OIDC)-based authentication from GitHub Actions to npm registry, which replaces long-lived access tokens.
- Why tokens/keys should not be used in CI: Trusted publishing provides cryptographically secure, short-lived credentials instead of persistent tokens, reducing attack surface.
- How it works: The npm CLI automatically detects the GitHub Actions CI environment, requests an OIDC token, and exchanges it for a short-lived npm publishing token—no manual
NPM_TOKENsetup required. - Initial release exception: The first version release requires manual npm CLI authentication (because the npm registry doesn't yet know about the package), but subsequent releases use trusted publishing configured in step 8.
Consider referencing npm's trusted publishing docs or expanding the explanation inline.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/source/internal/release-process.md` around lines 8 - 10, Expand the
trusted publishing section in the release-process.md file to provide
comprehensive context. Replace the current brief explanation with a more
detailed description that covers: what trusted publishing is (explain it as
OIDC-based authentication from GitHub Actions to npm registry that replaces
long-lived tokens), why tokens and keys should not be used in CI (clarify that
trusted publishing provides cryptographically secure short-lived credentials,
reducing attack surface), how the mechanism works (describe that npm CLI
automatically detects GitHub Actions environment, requests an OIDC token, and
exchanges it for a short-lived npm publishing token without manual NPM_TOKEN
setup), and the initial release exception workflow (clarify that the first
version release requires manual npm CLI authentication since the registry
doesn't yet know about the package, but subsequent releases use the trusted
publishing configured in step 8). Optionally reference the npm trusted
publishing documentation link or expand the explanation inline for clarity.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/source/internal/on-types.md (1)
3-3:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify ambiguous phrasing: "ignore calls" risks misinterpretation.
Line 3 uses "allow you to ignore calls to this library with arbitrary values," which is grammatically confusing. The word "ignore" could be misread as "skip making calls" rather than the intended meaning: "call with arbitrary values." This ambiguity was flagged in a prior review; the sentence still needs clarification for documentation clarity.
✏️ Suggested fix
-JavaScript allows you to ignore calls to this library with arbitrary values as arguments. +JavaScript allows you to call this library with arbitrary values as arguments.Or, more explicitly:
-JavaScript allows you to ignore calls to this library with arbitrary values as arguments. +JavaScript does not prevent you from calling this library with arbitrary values as arguments.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/internal/on-types.md` at line 3, The phrasing in line 3 of the on-types.md file is ambiguous. The phrase "allow you to ignore calls to this library with arbitrary values" could be misread as "skip making calls" rather than the intended meaning "make calls with arbitrary values." Replace "ignore calls to this library with arbitrary values" with clearer language that explicitly conveys that JavaScript allows you to call this library while passing arbitrary values as arguments, removing the ambiguity around the word "ignore."
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@docs/source/internal/on-types.md`:
- Line 3: The phrasing in line 3 of the on-types.md file is ambiguous. The
phrase "allow you to ignore calls to this library with arbitrary values" could
be misread as "skip making calls" rather than the intended meaning "make calls
with arbitrary values." Replace "ignore calls to this library with arbitrary
values" with clearer language that explicitly conveys that JavaScript allows you
to call this library while passing arbitrary values as arguments, removing the
ambiguity around the word "ignore."
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9fc3890b-8bc2-42f1-8150-d05471aab6ae
📒 Files selected for processing (4)
MAINTENANCE.mddocs/source/internal/SUMMARY.mddocs/source/internal/on-types.mddocs/source/internal/release-process.md
✅ Files skipped from review due to trivial changes (1)
- docs/source/internal/SUMMARY.md
wprzytula
reviewed
Jun 20, 2026
Include new step about docs, and remove last step replaced by CI step
We want to avoid breaking any additional releases, right?
wprzytula
approved these changes
Jun 20, 2026
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
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.
No description provided.