fix(scripts): remove static issue line from release PR template#1596
fix(scripts): remove static issue line from release PR template#1596brandonpelfrey merged 4 commits intomainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe bump-version script now updates/docs version artifacts ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ 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)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/bump-version.ts`:
- Around line 355-377: readDocsVersionsJson currently accepts any non-empty
string for historical entry.version, allowing malformed versions like "latest"
to be preserved; update readDocsVersionsJson to validate each candidate.version
using the same semver parsing/validation used by parseArgs() (or reuse the
parseArgs semver check function) and throw an error if the version is not a
valid semver; keep the existing URL logic (use candidate.url if valid otherwise
buildDocsVersionUrl(candidate.version)) and ensure preferred handling remains
unchanged so malformed versions cannot be carried forward.
🪄 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: CHILL
Plan: Pro
Run ID: a9245140-e067-4cc0-aea3-b23a002df558
📒 Files selected for processing (2)
docs/versions1.jsonscripts/bump-version.ts
✅ Files skipped from review due to trivial changes (1)
- docs/versions1.json
| function readDocsVersionsJson(): DocsVersionEntry[] { | ||
| try { | ||
| const parsed = JSON.parse(readText(DOCS_VERSIONS_JSON)) as unknown; | ||
| if (!Array.isArray(parsed)) { | ||
| throw new Error("docs/versions1.json must contain an array"); | ||
| } | ||
| return parsed.map((entry) => { | ||
| if (!entry || typeof entry !== "object") { | ||
| throw new Error("Invalid docs/versions1.json entry"); | ||
| } | ||
| const candidate = entry as Partial<DocsVersionEntry>; | ||
| if (typeof candidate.version !== "string") { | ||
| throw new Error("Each docs/versions1.json entry must include a string version"); | ||
| } | ||
| return { | ||
| preferred: candidate.preferred === true ? true : undefined, | ||
| version: candidate.version, | ||
| url: | ||
| typeof candidate.url === "string" && candidate.url.length > 0 | ||
| ? candidate.url | ||
| : buildDocsVersionUrl(candidate.version), | ||
| }; | ||
| }); |
There was a problem hiding this comment.
Validate retained docs versions with the same semver rule.
readDocsVersionsJson() carries forward every historical entry.version, but verifyDocsVersionsJson() only checks that it is a non-empty string. A malformed existing entry like "latest" would be rewritten with a matching URL and still pass verification, so corrupted docs metadata can survive a release bump. Reusing the parseArgs() semver check here would close that gap.
Proposed fix
+const SEMVER_RE =
+ /^[0-9]+\.[0-9]+\.[0-9]+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$/;
+
function parseArgs(args: string[]): Options {
...
- if (!/^[0-9]+\.[0-9]+\.[0-9]+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$/.test(version)) {
+ if (!SEMVER_RE.test(version)) {
throw new Error(`Invalid semver: ${version}`);
}
...
}- if (typeof candidate.version !== "string") {
- throw new Error("Each docs/versions1.json entry must include a string version");
+ if (typeof candidate.version !== "string" || !SEMVER_RE.test(candidate.version)) {
+ throw new Error("Each docs/versions1.json entry must include a valid semver version");
}- if (typeof entry.version !== "string" || entry.version.length === 0) {
+ if (typeof entry.version !== "string" || !SEMVER_RE.test(entry.version)) {
throw new Error(`docs/versions1.json entry ${index} is missing a valid version`);
}Also applies to: 622-657
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/bump-version.ts` around lines 355 - 377, readDocsVersionsJson
currently accepts any non-empty string for historical entry.version, allowing
malformed versions like "latest" to be preserved; update readDocsVersionsJson to
validate each candidate.version using the same semver parsing/validation used by
parseArgs() (or reuse the parseArgs semver check function) and throw an error if
the version is not a valid semver; keep the existing URL logic (use
candidate.url if valid otherwise buildDocsVersionUrl(candidate.version)) and
ensure preferred handling remains unchanged so malformed versions cannot be
carried forward.
mikemckiernan
left a comment
There was a problem hiding this comment.
Not an expert in the language, but lgtm! Thank you too!
Summary
Fix the release PR body generated by
scripts/bump-version.tsso it no longer includes a static issue-closing line for version bumps.This also corrects an oversight introduced in #1581 by making the test and formatting checkboxes reflect what the script actually ran.
Related Issue
Changes
Fixes #1577.line from the generated release PR bodybuildPrBody()accept runtime status sonpm testand formatting checklist items match the actual runType of Change
Testing
npx prek run --all-filespasses (or equivalentlymake check).npm testpasses.make docsbuilds without warnings. (for doc-only changes)Checklist
General
Code Changes
npx prek run --all-filesauto-fixes formatting (ormake formatfor targeted runs).Doc Changes
update-docsagent skill to draft changes while complying with the style guide. For example, prompt your agent with "/update-docscatch up the docs for the new changes I made in this PR."Signed-off-by: Brandon Pelfrey bpelfrey@nvidia.com
Summary by CodeRabbit
Documentation
Chores