Add support for saving properties to partials #140
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 extends the property documentation generation system with configurable generation modes controlled via CLI flags and environment variables. It introduces Sequence DiagramsequenceDiagram
participant CLI as doc-tools.js CLI
participant Env as Environment Variables
participant Gen as generate-handlebars-docs.js
participant Make as Makefile
CLI->>Env: Set GENERATE_PARTIALS, OUTPUT_PARTIALS_DIR,<br/>GENERATE_PAGES if flags provided
CLI->>Make: Invoke make-based tooling
Make->>Gen: Call generateAllDocs()
rect rgb(200, 220, 255)
Note over Gen: Check GENERATE_PARTIALS
alt GENERATE_PARTIALS === '1'
Gen->>Gen: generatePropertyPartials()
Gen->>Gen: Group properties by config_scope
Gen->>Gen: Write per-type partial files<br/>(cluster, topic, object-storage, broker)
else
Gen->>Gen: Skip partials generation
end
end
rect rgb(220, 200, 255)
Note over Gen: Check GENERATE_PAGES
alt GENERATE_PAGES === '1'
Gen->>Gen: generatePropertyDocs()<br/>per config_scope type
Gen->>Gen: Select template<br/>(with-includes vs full-content)
Gen->>Gen: Write complete pages
else
Gen->>Gen: Compute counts only,<br/>skip page generation
end
end
Gen->>Gen: Generate error reports<br/>(embedded in JSON)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes The changes span multiple interdependent systems: substantial refactoring of the core generation logic with new conditional branching, introduction of a new public function with property categorization logic, template selection based on generation mode, simplified error reporting flow, and modifications across multiple template files. The diversity of changes across CLI configuration, build tooling, core generation logic, and templates requires separate reasoning for each component despite consistent patterns in conditional logic. 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 (5)
tools/property-extractor/Makefile (1)
31-31: Expose OUTPUT_PARTIALS_DIR inmake checkfor easier debugging.Add it to the diagnostics so users can confirm where consolidated partials will land.
@@ check: @@ @echo "OUTPUT_JSON_DIR: $(OUTPUT_JSON_DIR)" @echo "OUTPUT_AUTOGENERATED_DIR: $(OUTPUT_AUTOGENERATED_DIR)" + @echo "OUTPUT_PARTIALS_DIR: $(OUTPUT_PARTIALS_DIR)"tools/property-extractor/generate-handlebars-docs.js (4)
210-229: Includes vs full template override: unify env var names.Currently this looks for TEMPLATE_PROPERTY_PAGE_WITH_INCLUDES, while the CLI sets TEMPLATE_PROPERTY_PAGE. Either accept both here or set both in the CLI (see suggested CLI fix).
- templatePath = getTemplatePath( - path.join(__dirname, 'templates', 'property-page-with-includes.hbs'), - 'TEMPLATE_PROPERTY_PAGE_WITH_INCLUDES' - ); + // Accept either env var for overrides + templatePath = getTemplatePath( + path.join(__dirname, 'templates', 'property-page-with-includes.hbs'), + process.env.TEMPLATE_PROPERTY_PAGE_WITH_INCLUDES ? 'TEMPLATE_PROPERTY_PAGE_WITH_INCLUDES' : 'TEMPLATE_PROPERTY_PAGE' + );
417-426: Deprecated partial path derivation is brittle; prefer OUTPUT_PARTIALS_DIR.String matching on 'pages/properties' can misroute on custom layouts. Use process.env.OUTPUT_PARTIALS_DIR if present, with a safe fallback.
- let outputPath; - if (outputDir.includes('pages/properties')) { - outputPath = path.join(path.dirname(path.dirname(outputDir)), 'partials', 'deprecated', 'deprecated-properties.adoc'); - } else { - outputPath = path.join(outputDir, 'partials', 'deprecated', 'deprecated-properties.adoc'); - } + const basePartialsDir = process.env.OUTPUT_PARTIALS_DIR + ? path.resolve(process.env.OUTPUT_PARTIALS_DIR) + : (outputDir.includes('pages') + ? path.resolve(outputDir, '..', '..', 'partials') + : path.join(outputDir, 'partials')); + const outputPath = path.join(basePartialsDir, 'deprecated', 'deprecated-properties.adoc');
63-74: Deduplicate object-storage classification logic.The name-based checks are repeated in PROPERTY_CONFIG, partials generation, and counting. Extract a single predicate to avoid drift.
// helpers/classification.js (example) function isObjectStorageProperty(name = '') { return /^(cloud_storage|s3_|azure_|gcs_|archival_|remote_|tiered_)/.test(name); } module.exports = { isObjectStorageProperty };Usage (illustrative):
- prop.name.includes('cloud_storage') || ... + isObjectStorageProperty(prop.name)Also applies to: 88-97, 334-346, 507-515
575-599: Deterministic error arrays.Sort the arrays before persisting to stabilize diffs and downstream consumers.
Object.values(properties).forEach(prop => { @@ }); - // Return the arrays so they can be added to the JSON output + // Stabilize output + emptyDescriptions.sort(); + deprecatedProperties.sort(); return { empty_descriptions: emptyDescriptions, deprecated_properties: deprecatedProperties };
📜 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)
bin/doc-tools.js(2 hunks)package.json(1 hunks)tools/property-extractor/Makefile(1 hunks)tools/property-extractor/generate-handlebars-docs.js(5 hunks)tools/property-extractor/templates/property-cloud.hbs(1 hunks)tools/property-extractor/templates/property-page-with-includes.hbs(1 hunks)tools/property-extractor/templates/property.hbs(1 hunks)
⏰ 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 (4)
package.json (1)
3-3: Version bump looks good.Please ensure a matching tag/release notes entry when merging.
tools/property-extractor/templates/property-cloud.hbs (1)
23-25: Good: BYOC note gated by env-cloud.Confirm env-cloud is set appropriately in Antora builds so the note appears only in Cloud docs.
tools/property-extractor/templates/property-page-with-includes.hbs (1)
1-17: Include-based page template LGTM.Path reference matches modules/reference/partials/properties by default.
tools/property-extractor/templates/property.hbs (1)
62-64: Helper is properly registered and available.The
renderPropertyExamplehelper is:
- Defined in
tools/property-extractor/helpers/renderPropertyExample.js- Exported from
tools/property-extractor/helpers/index.js(line 11)- Registered with Handlebars in
tools/property-extractor/generate-handlebars-docs.jsvia the loop at lines 22-28 that iterates through all exported helpers and callshandlebars.registerHelper(name, fn)The helper usage in property.hbs (and other template files) will work correctly.
2 participants
This pull request introduces a new system for generating consolidated AsciiDoc partials for Redpanda property documentation. The changes allow for more flexible documentation builds, including the ability to generate either full property pages, partials, or both, controlled using flags. The error reporting system is now integrated into the JSON output for better downstream consumption.
Property Documentation Generation Enhancements
generatePropertyPartialsto create consolidated AsciiDoc partials for each property type (cluster, topic, broker, object-storage), supporting both standard and cloud-aware templates. This enables easier reuse and inclusion of property documentation in other docs.generatePropertyDocsto conditionally select between a full property page template and an include-based template, allowing for modular documentation generation. [1] [2]generateAllDocs) now supports skipping full property pages and/or partials based on environment variables (GENERATE_PAGES,GENERATE_PARTIALS), improving build flexibility. [1] [2] [3]Error Reporting and Output Improvements
Template and Path Handling Updates
Version Bump
package.jsonfrom4.10.1to4.10.2.