-
Notifications
You must be signed in to change notification settings - Fork 8.5k
docs(extensions): generate variables table from schema #10557
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
base: main
Are you sure you want to change the base?
docs(extensions): generate variables table from schema #10557
Conversation
Summary of ChangesHello @HamdaanAliQuatil, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request automates the generation of the extension variables table within the documentation. By replacing a manually maintained table with content generated directly from the Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request automates the generation of the extension variables table in the documentation, ensuring it stays in sync with the variableSchema.ts
. A new script handles the generation, and CI checks are added to enforce this. The implementation is solid, but I've found a potential high-severity issue in the script where unescaped strings are used to construct a regular expression, which could lead to errors. I've provided suggestions to make the script more robust.
/gemini review |
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.
Code Review
This pull request introduces a script to automatically generate the extension variables table in the documentation from the TypeScript schema, which is a great improvement for keeping docs in sync with the code. The new npm scripts to generate and check the documentation are well-integrated into the development workflow. I've found one potential issue with the CI check that could cause friction for developers due to code formatting. My review includes a suggestion to address this.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
/gemini review |
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.
Code Review
This pull request introduces a great automation for keeping the extension variables documentation in sync with the source code schema. This is a valuable improvement for maintainability. I've added one comment on the new generator script to make it more robust by handling special characters and newlines in descriptions, which will prevent potential rendering issues in the documentation.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
/gemini review |
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.
Code Review
This pull request automates the generation of the extension variables table in the documentation, ensuring it stays in sync with the schema. This is an excellent improvement for maintainability. The script and CI checks are well-implemented. My only concern is a regression in the level of detail in the generated documentation compared to the previous, hand-written version. I have left a comment with a suggestion to address this.
not sure when is a good time to stop using @gemini-code-assist 😅 @chrstnb can you please take a look |
/gemini review |
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.
Code Review
This pull request introduces a script to automatically generate the extension variables table in the documentation from the TypeScript schema, which is an excellent improvement for keeping the documentation synchronized with the code. The implementation is clean and the accompanying npm scripts for generation and CI checks are well-structured. I've identified a potential issue regarding the use of trim()
on the file content, which could lead to unintended formatting changes. My review includes suggestions to make the file writing process more robust.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
/gemini review |
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.
Code Review
This pull request automates the generation of the supported variables table in the extensions documentation from the variableSchema.ts
file. This is a great initiative to prevent documentation from becoming outdated. A new script is added for generation, and npm scripts are wired up to run it and check for consistency in CI. My main feedback is that the new descriptions, sourced from the schema, are less detailed than the previous manual ones, which represents a regression in documentation quality. I've left a comment with a suggestion to improve the descriptions in the schema file to address this.
/gemini review |
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.
Code Review
This pull request introduces a script to automatically generate the supported variables table in the extension documentation from the variableSchema.ts
source of truth. This is a great improvement for maintainability, ensuring the documentation stays in sync with the code. The changes include the script itself, new npm commands to run and check it, and wiring it into CI.
My review focuses on the robustness of the new generation script. I've identified a potential issue where the script could corrupt the documentation file if the auto-generation markers are partially deleted. I've provided a suggestion to make the script more resilient to such scenarios.
function upsertBlock(original: string, block: string): string { | ||
if (original.includes(START_MARKER) && original.includes(END_MARKER)) { | ||
const pattern = new RegExp( | ||
`${escapeRegExp(START_MARKER)}[\\s\\S]*?${escapeRegExp(END_MARKER)}`, | ||
); | ||
return original.replace(pattern, block); | ||
} | ||
|
||
return `${original.replace(/\s*$/, '')}\n\n${block}\n`; | ||
} |
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.
The current implementation of upsertBlock
is not robust against partial deletion of the auto-generation markers. If only one of START_MARKER
or END_MARKER
is present in the document, the function will append a new block, leading to a malformed file with duplicate markers. This can cause confusion and further issues on subsequent runs.
To prevent this, the function should explicitly check for this inconsistent state and fail loudly by throwing an error. This ensures that any manual edits that break the marker pairing are caught immediately.
function upsertBlock(original: string, block: string): string {
const hasStart = original.includes(START_MARKER);
const hasEnd = original.includes(END_MARKER);
if (hasStart && hasEnd) {
const pattern = new RegExp(
`${escapeRegExp(START_MARKER)}[\s\S]*?${escapeRegExp(END_MARKER)}`,
);
return original.replace(pattern, block);
}
if (hasStart || hasEnd) {
throw new Error(
`Unbalanced auto-generation markers found in ${DOC_PATH}. Please fix it manually.`
);
}
return `${original.replace(/\s*$/, '')}\n\n${block}\n`;
}
TLDR
docs/extensions/index.md
with a generated blockdocs:gen:extensions
script so the table always matches the schema invariableSchema.ts
docs:check:extensions
step into CI so we catch drift automaticallyDive Deeper
PR #7035 introduced
VARIABLE_SCHEMA
, but the doc still had the older human-written wording.This change adds
scripts/docgen/generate-extension-vars.ts
, which pulls the schema directly and rewrites the table between HTML markers. The doc now uses the schema descriptions verbatim.CI’s lint job runs
npm run docs:check:extensions
that command regenerates the section and fails if the doc needs committing, which should keep future updates in sync.Reviewer Test Plan
npm run docs:gen:extensions
- verifies the generator runs clean with no extra changes.npm run docs:check:extensions
- should exit 0 because the doc is already up to date.No product behavior changes, so no runtime testing is required.
Testing Matrix
Linked issues / bugs
Closes #7072