Fix Redpanda Connect cloud-only connector automation #163
Conversation
✅ Deploy Preview for docs-extensions-and-macros ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis PR contains primarily textual and documentation updates across the codebase, focusing on standardizing pluralization in CLI help text, error messages, and documentation comments. Changes remove "(s)" notation in various messages (e.g., "surface(s)" becomes "surfaces", "error(s)" becomes "errors"). A minor comment is removed, and the package version is bumped from 4.13.2 to 4.13.3. The frontmatter attribute name in a README is updated from Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tools/redpanda-connect/rpcn-connector-docs-handler.js (2)
771-802: Redundant variable re-declaration on line 799.The
expectedPathvariable is already defined on line 773 with the same value. The re-declaration inside theif (tempRenamed)block is unnecessary.🔎 Proposed fix
// Restore the augmented file if (tempRenamed) { - const expectedPath = path.join(dataDir, `connect-${newVersion}.json`) fs.unlinkSync(expectedPath) fs.renameSync(path.join(dataDir, `._connect-${newVersion}-augmented.json.tmp`), expectedPath) }
941-975: Consider extractingdocRootsto avoid duplication.The
docRootsobject is defined identically on lines 942-946 and again on lines 970-975. Consider extracting it to a single definition earlier in the function scope.🔎 Proposed refactor
Define
docRootsonce before its first usage:+ // Define doc roots for checking existing documentation + const docRoots = { + pages: path.resolve(process.cwd(), 'modules/components/pages'), + partials: path.resolve(process.cwd(), 'modules/components/partials/components'), + cloudOnly: path.resolve(process.cwd(), 'modules/components/partials/components/cloud-only') + } + // Generate diff JSON let diffJson = null if (!oldVersion) { // ... } else { // ... - // Filter out components that already have documentation - const docRoots = { - pages: path.resolve(process.cwd(), 'modules/components/pages'), - partials: path.resolve(process.cwd(), 'modules/components/partials/components'), - cloudOnly: path.resolve(process.cwd(), 'modules/components/partials/components/cloud-only') - } if (diffJson.details && diffJson.details.newComponents) { // ... use docRoots } // Add new cgo-only components if (binaryAnalysis && binaryAnalysis.cgoOnly && binaryAnalysis.cgoOnly.length > 0) { - // Define roots for checking if docs already exist - const docRoots = { - pages: path.resolve(process.cwd(), 'modules/components/pages'), - partials: path.resolve(process.cwd(), 'modules/components/partials/components'), - cloudOnly: path.resolve(process.cwd(), 'modules/components/partials/components/cloud-only') - } // ... use docRoots
📜 Review details
Configuration used: Organization 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 (16)
CLI_REFERENCE.adoc__tests__/tools/cgo-detection.test.jsbin/doc-tools-mcp.jsbin/doc-tools.jsbin/mcp-tools/generated-docs-review.jsbin/mcp-tools/mcp-validation.jsbin/mcp-tools/openapi.jsextensions/generate-rp-connect-categories.jspackage.jsontools/bundle-openapi.jstools/redpanda-connect/README.adoctools/redpanda-connect/connector-binary-analyzer.jstools/redpanda-connect/pr-summary-formatter.jstools/redpanda-connect/report-delta.jstools/redpanda-connect/rpcn-connector-docs-handler.jstools/redpanda-connect/update-nav.js
💤 Files with no reviewable changes (1)
- extensions/generate-rp-connect-categories.js
🧰 Additional context used
🧬 Code graph analysis (3)
tools/redpanda-connect/connector-binary-analyzer.js (1)
__tests__/tools/cgo-detection.test.js (2)
cgoOnly(171-171)cgoOnly(364-364)
tools/redpanda-connect/rpcn-connector-docs-handler.js (2)
tools/redpanda-connect/generate-rpcn-connector-docs.js (1)
isCloudOnly(399-399)tools/redpanda-connect/report-delta.js (1)
oldCgoSet(232-232)
tools/redpanda-connect/pr-summary-formatter.js (2)
__tests__/tools/cgo-detection.test.js (1)
diffData(260-286)tools/redpanda-connect/connector-binary-analyzer.js (2)
inCloud(348-348)cloudOnly(350-350)
🔇 Additional comments (20)
tools/redpanda-connect/connector-binary-analyzer.js (1)
506-506: LGTM: Improved message clarity.The pluralization update improves message consistency and readability.
tools/bundle-openapi.js (1)
554-554: LGTM: Accurate documentation improvement.The plural form accurately reflects that this function can process multiple API surfaces ('admin', 'connect', or 'both').
__tests__/tools/cgo-detection.test.js (1)
396-396: LGTM: Consistent error messaging.The pluralization update aligns with the broader consistency improvements across the codebase.
package.json (1)
3-3: LGTM: Appropriate patch version bump.The patch version increment from 4.13.2 to 4.13.3 correctly follows semantic versioning for the textual improvements and bug fixes in this PR.
bin/mcp-tools/mcp-validation.js (1)
253-253: LGTM: Improved validation message clarity.The plural form is always correct at this point since the code path requires
results.errors.length > 0. This change improves consistency with the broader pluralization updates.bin/doc-tools-mcp.js (1)
365-365: LGTM! Terminology improvement.The change from "API surface(s)" to "API surfaces" improves readability and professionalism in the tool description. This aligns with the consistent pluralization effort across the codebase.
CLI_REFERENCE.adoc (1)
1077-1077: LGTM! Documentation consistency improvement.The pluralization change aligns with the updated terminology in the CLI tool definitions, maintaining consistency between the auto-generated reference and the actual implementation.
bin/mcp-tools/openapi.js (2)
19-19: LGTM! JSDoc clarity improvement.The parameter documentation change from "API surface(s)" to "API surfaces" improves readability while maintaining accuracy. This is part of the consistent pluralization effort across the codebase.
158-158: LGTM! Success message improvement.The change from "API(s)" to "APIs" in the success summary makes the output message more natural and professional. This aligns with the pluralization consistency improvements throughout the PR.
bin/mcp-tools/generated-docs-review.js (1)
309-312: LGTM! Report text clarity improvement.The removal of "(s)" notation in the Next Steps section makes the generated review reports more readable and professional. Using the plural form directly ("errors" and "warnings") is clearer since the actual count is displayed.
bin/doc-tools.js (1)
1633-1633: LGTM! CLI option description improvement.The change from "Which API surface(s) to bundle" to "Which API surfaces to bundle" improves the help text clarity. This is the source definition that propagates to the auto-generated CLI reference documentation, ensuring consistency across all user-facing documentation.
tools/redpanda-connect/README.adoc (1)
668-689: LGTM! Consistent attribute name standardization.The update from
:commercial-names:to:page-commercial-names:aligns with the PR's goal of standardizing attribute naming across the codebase.tools/redpanda-connect/update-nav.js (1)
193-209: LGTM! Clean filtering logic for cloud-only connectors.The implementation correctly excludes cloud-only connectors from navigation updates while providing clear logging for transparency. The filter logic is straightforward and handles edge cases (undefined
cloudOnlyproperty) via truthiness check.tools/redpanda-connect/report-delta.js (1)
250-255: LGTM! Consistent extension of binary analysis details.The addition of
cloudOnlyfollows the same pattern ascloudSupportedandselfHostedOnly, with proper optional chaining and fallback to an empty array.tools/redpanda-connect/pr-summary-formatter.js (3)
33-33: LGTM! Improved change detection logic.Good enhancement to consider
draftedConnectorsas changes even when all summary stats are zero. This ensures PRs with only drafted connectors are properly flagged as having changes.
94-99: LGTM! Attribute name standardization.Consistent update to use
:page-commercial-names:across writer guidance.
104-165: LGTM! Comprehensive cloud docs update section.The implementation correctly:
- Guards against missing
binaryAnalysisandbinaryAnalysis.comparison- Identifies cloud-supported connectors by checking both
inCloudandcloudOnly- Separates cloud-only connectors (needing partials syntax) from regular cloud connectors (needing page syntax)
- Provides clear actionable instructions for writers
The conditional checks for
cloudOnlyexistence (lines 116-117, 131-136) properly handle cases where the array may not be present.tools/redpanda-connect/rpcn-connector-docs-handler.js (3)
151-173: LGTM! Proper filtering of cloud-only connectors from whats-new updates.The logic correctly:
- Filters out cloud-only connectors from the whats-new section (they shouldn't appear in OSS release notes)
- Uses the updated path
diff.binaryAnalysis?.details?.cgoOnlyfor CGO detection
664-683: LGTM! Clean OSS data generation for accurate binary analysis.Creating a stripped copy of the connector data (removing
cloudOnlyconnectors and augmentation fields) ensures the binary analyzer compares actual binary contents rather than previously augmented data. This prevents false positives in the comparison.
899-903: LGTM! Consistent pluralization and log message improvements.The log message updates correctly pluralize terms ("connectors" vs "connector") and standardize phrasing across the codebase.
Also applies to: 1008-1013, 1041-1041
This pull request introduces several improvements and bug fixes to the Redpanda connector documentation tooling, primarily focused on handling cloud-only connectors, updating documentation instructions, and improving binary analysis accuracy. The changes ensure that cloud-only connectors are properly excluded from navigation and certain documentation outputs, instructions for writers are updated to use the correct attribute name, and the binary analysis process now uses unaugmented data for more reliable results.
Cloud-only connector handling:
update-nav.js, with clear logging for skipped items.Binary analysis improvements:
Documentation and writer instructions:
:page-commercial-names:instead of:commercial-names:in README and PR summary outputs. [1] [2] [3] [4]General bug fixes and improvements:
Version bump:
4.13.2to4.13.3inpackage.json.