Add support for excluding properties from docs (using Asciidoc tags) #142
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 refactors the property documentation generation tooling by introducing a partials-focused workflow and enhanced property categorization features. Key changes include: simplifying the CLI (removing template-property-page and generate-pages options, adding --template-topic-property-mappings), refactoring the build flow to handle old/new tag sequencing for diffs, removing cloud-specific templates (property-cloud.hbs, topic-property-cloud.hbs), introducing topic-property-mappings template generation, adding support for property exclusion and category metadata in the extraction pipeline, and creating an anchorName helper utility. Description truncation in comparison reports is removed. The public API of generate-handlebars-docs.js is updated (three functions exported, two removed). Sequence DiagramsequenceDiagram
participant CLI as bin/doc-tools.js
participant Extractor as property_extractor.py
participant TemplateEngine as generate-handlebars-docs.js
participant Templates as Handlebars Templates
CLI->>CLI: Parse --template-topic-property-mappings
Note over CLI: Removed: --template-property-page<br/>--generate-pages
alt oldTag provided (diff mode)
CLI->>Extractor: Build old version
Extractor->>Extractor: Apply category inference<br/>Apply exclude_from_docs override
end
CLI->>Extractor: Build new version
Extractor->>Extractor: Extract properties<br/>+ category + exclude_from_docs
Extractor->>TemplateEngine: Pass properties with new metadata
TemplateEngine->>TemplateEngine: generatePropertyPartials()
TemplateEngine->>Templates: Render property.hbs<br/>(with category/exclude_from_docs tags)
TemplateEngine->>Templates: Render topic-property.hbs<br/>(with conditional blocks)
TemplateEngine->>Templates: Render topic-property-mappings.hbs<br/>(NEW: topic→cluster mapping)
TemplateEngine->>Templates: generateDeprecatedDocs()
Note over Templates: Removed:<br/>property-cloud.hbs<br/>topic-property-cloud.hbs<br/>property-page.hbs<br/>property-page-with-includes.hbs
TemplateEngine-->>CLI: Return generated partials
CLI->>CLI: Compare old/new JSONs (if both present)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes This PR involves significant heterogeneous changes across multiple subsystems: CLI refactoring (bin/doc-tools.js with parameter renames and new options), public API modifications (generate-handlebars-docs.js exports), Python property extraction logic (category inference and override handling), template reorganization (four deletions, structural changes to renderers), and helper utility additions. The logic density is moderate-to-high, particularly in template changes and build flow sequencing. File deletions require verification that no external dependencies remain. Public API changes demand careful review for correctness and compatibility. 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 (7)
tools/property-extractor/property_extractor.py (1)
1667-1720: Consider extracting the category mapping for maintainability.The
infer_categoryfunction is defined inline with hardcoded property-name-to-category mappings. While functional, this approach may become difficult to maintain as the number of categories and properties grows.Consider extracting the category mappings to a module-level constant or external configuration:
+# Topic property category mappings +TOPIC_PROPERTY_CATEGORIES = { + "retention-compaction": [ + "cleanup.policy", "compaction.strategy", "delete.retention.ms", + "max.compaction.lag.ms", "min.cleanable.dirty.ratio", + "min.compaction.lag.ms", "retention.bytes", "retention.ms" + ], + "segment-message": [ + "compression.type", "max.message.bytes", "message.timestamp.type", + "segment.bytes", "segment.ms" + ], + # ... other categories +} + +def infer_category(name): + """Infer category for a topic property based on its name.""" + for category, properties in TOPIC_PROPERTY_CATEGORIES.items(): + if name in properties: + return category + return "other" + def extract_topic_properties(source_path): # ... - def infer_category(name): - retention = [...] - # ...This refactor would:
- Improve readability by separating data from logic
- Make it easier to add/modify categories
- Enable potential future externalization to a YAML/JSON config file
tools/property-extractor/helpers/anchorName.js (1)
1-3: Consider adding defensive checks for edge cases.The implementation correctly normalizes property names for anchor generation, but doesn't handle edge cases where the result might be an empty string.
Consider adding validation for edge cases:
module.exports = function anchorName(name) { - return String(name).replace(/[^a-zA-Z0-9]/g, '').toLowerCase(); + const anchor = String(name).replace(/[^a-zA-Z0-9]/g, '').toLowerCase(); + if (!anchor) { + throw new Error(`Invalid property name for anchor generation: "${name}"`); + } + return anchor; };This would catch cases where a property name contains only special characters, preventing silent failures that could result in invalid AsciiDoc anchor references.
tools/property-extractor/templates/property.hbs (2)
2-3: Stabilize tag/anchor identifiers (use helper) and add explicit anchor ID.Use anchorName to ensure valid tag IDs and cross‑refs; also emit an explicit anchor for headings.
Apply:
-// tag::category-{{category}}[] +// tag::category-{{anchorName category}}[] -=== {{name}} +[[{{anchorName name}}]] +=== {{name}} -// end::category-{{category}}[] +// end::category-{{anchorName category}}[]Also applies to: 116-118, 13-13
86-86: Make Nullable self‑managed‑only (align with topic template).Topic template wraps Nullable with self‑managed‑only; mirror here for consistency.
-*Nullable:* {{#if nullable}}Yes{{else}}No{{/if}} +// tag::self-managed-only[] +*Nullable:* {{#if nullable}}Yes{{else}}No{{/if}} +// end::self-managed-only[]tools/property-extractor/templates/topic-property.hbs (2)
2-3: Use anchorName for tag IDs and xref anchors; add explicit anchor ID.Prevents invalid IDs and broken links.
-// tag::category-{{category}}[] +// tag::category-{{anchorName category}}[] -=== {{name}} +[[{{anchorName name}}]] +=== {{name}} -*Related cluster property:* xref:reference:cluster-properties.adoc#{{corresponding_cluster_property}}[{{corresponding_cluster_property}}] +*Related cluster property:* xref:reference:cluster-properties.adoc#{{anchorName corresponding_cluster_property}}[{{corresponding_cluster_property}}] -// end::category-{{category}}[] +// end::category-{{anchorName category}}[]Also applies to: 107-109, 13-13, 45-46
31-34: Guard BYOC note with env‑cloud for consistency.Property template shows the BYOC note only in cloud builds; mirror that here.
-{{#if cloud_byoc_only}} - -NOTE: This property is only available in Redpanda Cloud BYOC deployments. -{{/if}} +{{#if cloud_byoc_only}} +ifdef::env-cloud[] +NOTE: This property is only available in Redpanda Cloud BYOC deployments. +endif::[] +{{/if}}tools/property-extractor/generate-handlebars-docs.js (1)
203-207: Prefer config_scope for topic filter (or verify is_topic_property is always set).Current filter depends on is_topic_property; safer to rely on config_scope to avoid missing mappings.
- const topicProperties = Object.values(properties).filter( - p => p.is_topic_property && p.corresponding_cluster_property - ); + const topicProperties = Object.values(properties).filter( + p => p.config_scope === 'topic' && p.corresponding_cluster_property + );If is_topic_property is guaranteed, keep as-is; otherwise adopt the change. Can you confirm upstream guarantees? I can adjust accordingly.
📜 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 (16)
__tests__/docs-data/property-overrides-exclude-test.json(1 hunks)bin/doc-tools.js(4 hunks)package.json(1 hunks)tools/property-extractor/compare-properties.js(3 hunks)tools/property-extractor/generate-handlebars-docs.js(4 hunks)tools/property-extractor/helpers/anchorName.js(1 hunks)tools/property-extractor/helpers/index.js(1 hunks)tools/property-extractor/property_extractor.py(4 hunks)tools/property-extractor/templates/deprecated-property.hbs(1 hunks)tools/property-extractor/templates/property-cloud.hbs(0 hunks)tools/property-extractor/templates/property-page-with-includes.hbs(0 hunks)tools/property-extractor/templates/property-page.hbs(0 hunks)tools/property-extractor/templates/property.hbs(2 hunks)tools/property-extractor/templates/topic-property-cloud.hbs(0 hunks)tools/property-extractor/templates/topic-property-mappings.hbs(1 hunks)tools/property-extractor/templates/topic-property.hbs(4 hunks)
💤 Files with no reviewable changes (4)
- tools/property-extractor/templates/property-page-with-includes.hbs
- tools/property-extractor/templates/property-page.hbs
- tools/property-extractor/templates/property-cloud.hbs
- tools/property-extractor/templates/topic-property-cloud.hbs
🧰 Additional context used
🧬 Code graph analysis (2)
tools/property-extractor/helpers/anchorName.js (1)
bin/doc-tools.js (1)
name(1292-1292)
bin/doc-tools.js (1)
tools/bundle-openapi.js (4)
options(475-475)options(581-581)path(4-4)tempDir(596-596)
⏰ 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 (8)
package.json (1)
3-3: LGTM! Appropriate patch version bump.The version increment from 4.10.3 to 4.10.4 correctly reflects a patch release for the property extraction and templating enhancements.
__tests__/docs-data/property-overrides-exclude-test.json (1)
1-13: LGTM! Well-structured test data.The test file appropriately covers both positive (admin with
exclude_from_docs: true) and negative (cloud_storage_access_key without exclusion) cases for the new exclusion feature.tools/property-extractor/compare-properties.js (1)
151-151: LGTM! Description truncation appropriately removed.The change from truncated descriptions to full descriptions is consistent across all property comparison contexts (new, changed, and removed properties), aligning with the broader template changes that favor complete property information.
Also applies to: 188-189, 206-206
tools/property-extractor/property_extractor.py (2)
722-728: LGTM! Consistent override handling for new metadata fields.The addition of
exclude_from_docsandcategoryoverride support properly extends the existing override mechanism for topic property metadata.
769-775: LGTM! Consistent metadata support in property creation.The
exclude_from_docsandcategoryfields are correctly propagated when creating new properties from overrides.tools/property-extractor/helpers/index.js (1)
13-13: LGTM! Helper export follows existing pattern.The
anchorNamehelper is correctly exported and follows the established pattern used by other helpers in this module.tools/property-extractor/templates/deprecated-property.hbs (1)
1-1: LGTM! Simplified template aligns with refactored workflow.The template now renders deprecated properties as list items rather than full sections, which is consistent with the broader template restructuring described in the PR.
tools/property-extractor/templates/topic-property-mappings.hbs (1)
1-13: LGTM! Well-structured template with consistent data validation.The AsciiDoc table syntax is correct, and the template properly uses the
anchorNamehelper for generating anchors andxreffor cross-references. The inline comments are helpful for understanding the helper's behavior.Verification confirms that the
corresponding_cluster_propertyfield is consistently populated for all topic properties rendered by this template. The JavaScript code (generate-handlebars-docs.js:204-214) filters properties to include only those with bothis_topic_propertyand a truthycorresponding_cluster_propertyvalue before passing the filtered array to this template. This guarantees that every property in the {{#each}} loop has the field populated, making the direct field access in the template safe.
2 participants
This pull request introduces improvements to the property extraction and documentation generation process, focusing on enhanced support for property categorization, exclusion from documentation, and more flexible templating for property pages. It also adds new helpers and refines how property descriptions are handled in change reports.
Property categorization and exclusion support:
categoryto topic properties based on their name patterns, and to propagate theexclude_from_docsflag from overrides to the property data model inproperty_extractor.py. [1] [2] [3] [4]property.hbs,topic-property.hbs) to include new AsciiDoc tags forcategory,exclude_from_docs, anddeprecated, and to conditionally render these tags in the output. [1] [2] [3] [4] [5] [6]Templating and documentation improvements:
topic-property-mappings.hbs) which uses anchor links for easier cross-referencing in documentation.Helper and utility enhancements:
anchorNamehelper for generating anchor-friendly property names, and integrated it into the helpers index and templates. [1] [2] [3]Change reporting improvements:
compare-properties.jsto use full property descriptions instead of truncated versions, improving clarity in change logs. [1] [2] [3]Testing and versioning:
exclude_from_docsfunctionality, and bumped the package version to4.10.4. [1] [2]