-
Notifications
You must be signed in to change notification settings - Fork 3
Enable bundle-openapi CLI to process any branch #144
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
Conversation
✅ Deploy Preview for docs-extensions-and-macros ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe pull request includes a version bump in package.json from 4.10.5 to 4.10.6. In tools/bundle-openapi.js, the Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes The diff involves a trivial version bump and straightforward logic modifications to two functions that adjust error handling and branch name processing. The changes are localized to a single utility file, follow a consistent pattern (handling edge cases more gracefully), and introduce no new exported signatures or complex interdependencies. Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tools/bundle-openapi.js (2)
9-17: Update the docstring to reflect the new behavior.The function documentation is outdated:
- Line 11 mentions "returns 'dev' unchanged" as a special case, but the implementation now returns any non-semver input unchanged (not just 'dev').
- Line 17 states "Throws if...does not conform to the expected version format," but the implementation now returns non-semver inputs as-is without throwing.
Apply this diff to update the documentation:
/** * Normalize a Git tag into a semantic version string. * - * Trims surrounding whitespace, returns 'dev' unchanged, removes a leading 'v' if present, - * and validates that the result matches MAJOR.MINOR.PATCH with optional pre-release/build metadata. - * Throws if the input is not a non-empty string or does not conform to the expected version format. + * Trims surrounding whitespace, removes a leading 'v' if present for semantic versions, + * and validates that the result matches MAJOR.MINOR.PATCH with optional pre-release/build metadata. + * Non-semantic version strings (e.g., branch names) are returned as-is. + * Throws if the input is not a non-empty string. * - * @param {string} tag - Git tag (e.g., 'v25.1.1', '25.1.1', or 'dev'). - * @returns {string} Normalized version (e.g., '25.1.1' or 'dev'). - * @throws {Error} If `tag` is not a non-empty string or does not match the semantic version pattern. + * @param {string} tag - Git tag or branch name (e.g., 'v25.1.1', '25.1.1', 'dev', 'enable-branches'). + * @returns {string} Normalized version for semver inputs (e.g., '25.1.1') or original tag for non-semver inputs (e.g., 'dev'). + * @throws {Error} If `tag` is not a non-empty string. */
42-50: Update the docstring to reflect the new behavior.The function documentation is outdated. Lines 45-46 state "The special value
'dev'is returned unchanged," but the implementation now returns any non-semver input unchanged, not just 'dev'. The function now treats all non-semver inputs as branch names.Apply this diff to update the documentation:
/** * Return the major.minor portion of a semantic version string. * - * Accepts a semantic version like `25.1.1` and yields `25.1`. The special value - * `'dev'` is returned unchanged. - * @param {string} version - Semantic version (e.g., `'25.1.1'`) or `'dev'`. - * @returns {string} The `major.minor` string (e.g., `'25.1'`) or `'dev'`. + * Accepts a semantic version like `25.1.1` and yields `25.1`. Non-semantic version + * inputs (e.g., branch names) are returned unchanged. + * @param {string} version - Semantic version (e.g., `'25.1.1'`) or branch name (e.g., `'dev'`, `'enable-branches'`). + * @returns {string} The `major.minor` string (e.g., `'25.1'`) for semver inputs, or the original input for non-semver inputs. * @throws {Error} If `version` is not a non-empty string, lacks major/minor parts, or if major/minor are not numeric. */
🧹 Nitpick comments (1)
tools/bundle-openapi.js (1)
57-57: Consider extracting the duplicate semver pattern.The semver pattern
/^\d+\.\d+\.\d+(-[\w\.-]+)?(\+[\w\.-]+)?$/is duplicated in bothnormalizeTag(line 34) andgetMajorMinor(line 57). Consider extracting it to a module-level constant to ensure consistency and ease maintenance.Add at the top of the file (after requires):
const SEMVER_PATTERN = /^\d+\.\d+\.\d+(-[\w\.-]+)?(\+[\w\.-]+)?$/;Then update both functions:
function normalizeTag(tag) { // ... validation code ... const normalized = tag.startsWith('v') ? tag.slice(1) : tag; - const semverPattern = /^\d+\.\d+\.\d+(-[\w\.-]+)?(\+[\w\.-]+)?$/; - if (semverPattern.test(normalized)) { + if (SEMVER_PATTERN.test(normalized)) { return normalized; } return tag; } function getMajorMinor(version) { // ... validation code ... - const semverPattern = /^\d+\.\d+\.\d+(-[\w\.-]+)?(\+[\w\.-]+)?$/; - if (!semverPattern.test(version)) { + if (!SEMVER_PATTERN.test(version)) { return version; } // ... rest of function ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.json(1 hunks)tools/bundle-openapi.js(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: Test Extensions and Macros
package.json
[error] 1-1: Process completed with exit code 1.
tools/bundle-openapi.js
[error] 216-216: OpenAPI bundling encountered a permission denied error while reading a directory during fragment discovery.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - docs-extensions-and-macros
- GitHub Check: Header rules - docs-extensions-and-macros
- GitHub Check: Pages changed - docs-extensions-and-macros
🔇 Additional comments (3)
package.json (1)
3-3: LGTM!The version bump from 4.10.5 to 4.10.6 is appropriate for this patch release.
tools/bundle-openapi.js (2)
35-39: Verify handling of semver-like branch names.The current logic treats any input matching the semver pattern (including pre-release suffixes like
-dev,-alpha) as a semantic version and strips thevprefix. This means a branch name likev25.1.1-dev-branchwould be normalized to25.1.1-dev-branchand treated as a semver rather than a branch name.Please confirm this is the intended behavior. If branch names can follow semver-like patterns but should still be treated as branch names, additional logic may be needed to distinguish them.
205-218: No issues found with the debug logging code.The verification test confirms that the
vbuilddirectory and its contents have standard permissions (drwxr-xr-x for directories, -rw-r--r-- for files) and the recursive directory read completes successfully without permission denied errors. The code at lines 205-218 already includes proper error handling with a try-catch block that logs any errors gracefully.The pipeline error mentioned in the original review comment appears to be either transient, environment-specific to the CI system, or stemming from a different code section. The logging code itself is functioning as intended.
kbatuigas
left a comment
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.
🚀
Allows any branch name (not just "dev") to be used as a version, and only applies semantic version parsing when appropriate.
Version normalization and parsing improvements:
normalizeTagintools/bundle-openapi.jsto remove special handling for thedevbranch and allow any non-semver tag to be treated as a branch name, returning it as-is.getMajorMinorintools/bundle-openapi.jsto only extract major and minor versions if the input is a valid semantic version, otherwise returning the branch name unchanged.Package metadata update:
package.jsonfrom4.10.5to4.10.6.