Support related topics for cloud/self-managed only #147
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 📝 WalkthroughWalkthroughThis PR updates the property-extractor tooling to support cloud-aware rendering of documentation. It bumps the version to 4.10.8 and adds cloud/self-managed environment-specific rendering capabilities. Key changes include: two new helper functions ( Sequence DiagramsequenceDiagram
participant Main as generate-handlebars-docs.js
participant PartialGen as generatePropertyPartials
participant Template as property.hbs / topic-property.hbs
participant Helper1 as allTopicsConditional
participant Helper2 as parseRelatedTopic
Main->>PartialGen: generatePropertyPartials(properties, dir)
PartialGen->>PartialGen: batch by property groups
PartialGen->>PartialGen: accumulate documented props
PartialGen->>Template: render each group
Template->>Helper1: allTopicsConditional(related_topics)
Helper1-->>Template: sectionType (cloud/self-managed/normal)
Template->>Helper2: parseRelatedTopic(each topic)
Helper2-->>Template: {type, value}
Template-->>PartialGen: rendered partial
PartialGen-->>Main: written files + documentedProps[]
Main->>Main: compute undocumented properties
Main->>Main: call generateErrorReports(properties, documentedProps)
Main-->>Main: error report with undocumented list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 minutes
Possibly related PRs
Suggested reviewers
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
tools/property-extractor/generate-handlebars-docs.js (5)
25-32: Avoid process.exit in library code; throw instead.Fail fast, but don’t terminate the host process from a module. Throwing keeps behavior consistent in both CLI and programmatic use.
Object.entries(helpers).forEach(([name, fn]) => { if (typeof fn !== 'function') { - console.error(`❌ Helper "${name}" is not a function`); - process.exit(1); + throw new TypeError(`Helper "${name}" is not a function`); } handlebars.registerHelper(name, fn); });
110-163: Deduplicate partial-generation logic; collect documentedProps via callback.Inline generation (Lines 279-315) duplicates generatePropertyPartials. Refactor generatePropertyPartials to accept an onRender callback that records names, then call it from generateAllDocs. Less drift, single source of truth.
-function generatePropertyPartials(properties, partialsDir) { +function generatePropertyPartials(properties, partialsDir, onRender /* (name: string) => void */) { @@ - Object.entries(propertyGroups).forEach(([type, props]) => { + Object.entries(propertyGroups).forEach(([type, props]) => { if (props.length === 0) return; props.sort((a, b) => String(a.name || '').localeCompare(String(b.name || ''))); const selectedTemplate = type === 'topic' ? topicTemplate : propertyTemplate; - const content = props.map(p => selectedTemplate(p)).join('\n'); + const content = props.map(p => { + if (typeof onRender === 'function' && p?.name) onRender(p.name); + return selectedTemplate(p); + }).join('\n'); const filename = `${type}-properties.adoc`; fs.writeFileSync(path.join(propertiesPartialsDir, filename), AUTOGEN_NOTICE + content, 'utf8'); console.log(`✅ Generated ${filename} (${props.length} properties)`); totalCount += props.length; }); @@ - return totalCount; + return totalCount; }- // Wrap generatePropertyPartials to also collect property names - const originalWrite = fs.writeFileSync; - const propertyTemplate = handlebars.compile( - fs.readFileSync(getTemplatePath(path.join(__dirname, 'templates', 'property.hbs'), 'TEMPLATE_PROPERTY'), 'utf8') - ); - const topicTemplate = handlebars.compile( - fs.readFileSync(getTemplatePath(path.join(__dirname, 'templates', 'topic-property.hbs'), 'TEMPLATE_TOPIC_PROPERTY'), 'utf8') - ); - - const propertiesPartialsDir = path.join(process.env.OUTPUT_PARTIALS_DIR, 'properties'); - fs.mkdirSync(propertiesPartialsDir, { recursive: true }); - - const propertyGroups = { cluster: [], topic: [], broker: [], 'object-storage': [] }; - Object.values(properties).forEach(p => { - if (!p.name || !p.config_scope) return; - if (p.config_scope === 'topic') propertyGroups.topic.push(p); - else if (p.config_scope === 'broker') propertyGroups.broker.push(p); - else if (p.config_scope === 'cluster') { - if (isObjectStorageProperty(p)) propertyGroups['object-storage'].push(p); - else propertyGroups.cluster.push(p); - } - }); - - Object.entries(propertyGroups).forEach(([type, props]) => { - if (props.length === 0) return; - props.sort((a, b) => String(a.name || '').localeCompare(String(b.name || ''))); - const selectedTemplate = type === 'topic' ? topicTemplate : propertyTemplate; - const content = props.map(p => { - documentedProps.push(p.name); - return selectedTemplate(p); - }).join('\n'); - const filename = `${type}-properties.adoc`; - originalWrite(path.join(propertiesPartialsDir, filename), AUTOGEN_NOTICE + content, 'utf8'); - console.log(`✅ Generated ${filename} (${props.length} properties)`); - partialsCount += props.length; - }); + // Use the single implementation and collect documented property names + partialsCount = generatePropertyPartials( + properties, + process.env.OUTPUT_PARTIALS_DIR, + (name) => documentedProps.push(name) + );Also applies to: 279-315
128-145: Scope grouping looks good; verify deprecation handling.Grouping via switch is clear and robust (incl. object-storage). Confirm whether deprecated properties should appear in these main partials; if not, filter them out when grouping.
- Object.values(properties).forEach(prop => { - if (!prop.name || !prop.config_scope) return; + Object.values(properties).forEach(prop => { + if (!prop.name || !prop.config_scope || prop.is_deprecated) return;Also applies to: 150-159
228-259: Error report expansion is solid; consider key-name disambiguation.Empty/Deprecated/Undocumented metrics and logs are clear. For repos where property keys may differ from p.name, optionally include both for easier follow-up.
- Object.entries(properties).forEach(([key, p]) => { - const name = p.name || key; + Object.entries(properties).forEach(([key, p]) => { + const name = p.name || key; if (!p.description || !p.description.trim()) emptyDescriptions.push(name); if (p.is_deprecated) deprecatedProperties.push(name); if (!documentedSet.has(name)) undocumented.push(name); });Optionally return tuples for undocumented to show key→name mapping:
- undocumented_properties: undocumented.sort(), + undocumented_properties: undocumented.sort(), + // optionally: undocumented_pairs: undocumentedPairs.sort(([a],[b]) => a.localeCompare(b)),
325-349: Compute coverage using documentedProps length for resilience.If templates ever skip rendering certain items, basing the percentage on documentedProps is more accurate than partialsCount.
- const pctRendered = totalProperties - ? ((partialsCount / totalProperties) * 100).toFixed(2) - : '0.00'; + const pctRendered = totalProperties + ? ((documentedProps.length / totalProperties) * 100).toFixed(2) + : '0.00';tools/property-extractor/helpers/allTopicsConditional.js (1)
1-19: Classification logic looks solid; minor maintainability nitWorks as intended for mixed/invalid inputs. Consider centralizing the two prefix strings in a shared constant to avoid drift with parseRelatedTopic.
tools/property-extractor/templates/topic-property.hbs (1)
82-124: Conditional rendering is correct; consider suppressing empty headersThe branching via allTopicsConditional + per-item parse is sound. With the parse fix, bullets won’t render for invalid/blank items. Optionally, suppress the “Related topics:” header if no items render (e.g., add a tiny helper like hasRenderableTopics to pre-check).
Please spot-check with samples:
- All cloud-only
- All self-managed-only
- Mixed (including blanks/non-strings)
- Empty array
tools/property-extractor/templates/property.hbs (1)
95-137: Related topics branching LGTM; optional empty-header guardMirrors topic-property.hbs correctly. After the parse fix, bullets won’t print for invalid/blank entries. Optionally add a pre-check to avoid rendering the “Related topics:” header when no items will display.
📜 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 (7)
package.json(1 hunks)tools/property-extractor/generate-handlebars-docs.js(7 hunks)tools/property-extractor/helpers/allTopicsConditional.js(1 hunks)tools/property-extractor/helpers/index.js(1 hunks)tools/property-extractor/helpers/parseRelatedTopic.js(1 hunks)tools/property-extractor/templates/property.hbs(3 hunks)tools/property-extractor/templates/topic-property.hbs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tools/property-extractor/helpers/parseRelatedTopic.js (1)
tools/property-extractor/helpers/allTopicsConditional.js (1)
trimmed(12-12)
tools/property-extractor/helpers/allTopicsConditional.js (1)
tools/property-extractor/helpers/parseRelatedTopic.js (1)
trimmed(5-5)
⏰ 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 (6)
package.json (1)
3-3: Verify that the semver bump aligns with the API changes.The version is bumped from 4.10.7 to 4.10.8 (patch), which typically signals backward-compatible changes. However, the PR summary indicates new exported helper functions (
parseRelatedTopic,allTopicsConditional) and changes to thegenerateErrorReportssignature. Please confirm:
- Are the new helpers added to the package exports? If so, this may warrant a minor version bump (4.11.0).
- Is the
generateErrorReportssignature change backward-compatible, or does it constitute a breaking change requiring a major bump?- Is
generatePropertyPartialsa new export? If so, this also suggests a minor version bump.tools/property-extractor/generate-handlebars-docs.js (3)
156-156: LGTM: Writes grouped partials with AUTOGEN notice.File creation and encoding are correct; directory is pre-created.
194-194: LGTM: Deprecated properties output path and write.Write location respects OUTPUT_PARTIALS_DIR; prepend AUTOGEN notice; good.
200-200: Nit: Header clarified.Descriptive comment improves readability.
tools/property-extractor/templates/property.hbs (1)
35-40: BYOC note wording change is fineText-only improvement; no behavioral impact.
tools/property-extractor/helpers/index.js (1)
14-15: All set; helpers are automatically registeredBoth
parseRelatedTopicandallTopicsConditionalare correctly exported and will be automatically registered through the existingObject.entries(helpers).forEach()pattern intools/property-extractor/generate-handlebars-docs.js(line 27). Both helpers are already in active use across the Handlebars templates, confirming the wiring is complete.
This pull request introduces several improvements and fixes to the property extraction and documentation generation tooling. The main changes focus on more accurate property type detection, improved handling of related topics in documentation, enhanced error reporting and statistics, and better parsing of C++ patterns for property values.
Property extraction and documentation generation improvements:
redpanda.remote.readreplicais now treated as a string, whileredpanda.iceberg.deleteis treated as a boolean. Additionally, the "Accepted values" section is now omitted for boolean properties. [1] [2]generateAllDocs, including tracking and reporting undocumented properties, percentage of properties rendered, and improved summary output. Error reports now include properties missing descriptions, deprecated properties, and those not documented. [1] [2] [3]std::chronoliterals are parsed and rendered in a concise format (e.g.,std::chrono::minutes{5}→"5 min"). Enum-like patterns are now more robustly detected. [1] [2] [3]allTopicsConditional,parseRelatedTopic) enable conditional rendering of related topics for cloud/self-managed/normal contexts, and templates are updated to use these helpers for both property and topic-property documentation. [1] [2] [3] [4] [5]Other:
package.jsonfrom4.10.7to4.10.8.