Skip to content

Conversation

@Joibel
Copy link
Member

@Joibel Joibel commented Dec 3, 2025

Motivation

We've got a lot of messy comments coming through into executor_swagger.md these days. The few we used to allow through have been supplemented by a lot of kubebuilder text.

Modifications

Add a Python script to make the generated executor swagger documentation more readable by converting annotations to a human-readable "Validation" section:

  • +optional → Validation: Optional.
  • +default=X → Validation: Default: X.
  • +kubebuilder:validation:Minimum=X → Validation: Minimum: X.
  • +kubebuilder:validation:Maximum=X → Validation: Maximum: X.
  • +kubebuilder:validation:MinLength=X → Validation: Minimum length: X.
  • +kubebuilder:validation:MaxLength=X → Validation: Maximum length: X.
  • +kubebuilder:validation:MinItems=X → Validation: Minimum items: X.
  • +kubebuilder:validation:MaxItems=X → Validation: Maximum items: X.
  • +kubebuilder:validation:Pattern=X → Validation: Pattern: X.
  • +kubebuilder:validation:Enum=A;B → Validation: Valid values: A, B.

Internal annotations are removed: +patchStrategy, +patchMergeKey, +listType, +listMapKey, +featureGate, +structType, +protobuf, +union, +enum, +kubebuilder:validation:XValidation (complex CEL rules).

Verification

NA

Documentation

Yup, this is all about documentation

Summary by CodeRabbit

  • Documentation
    • Enhanced Swagger-generated documentation formatting by converting internal annotations into human-readable validation sections, improving clarity and readability of API documentation.

✏️ Tip: You can customize this high-level summary in your review settings.

Add a Python script to make the generated executor swagger documentation
more readable by converting annotations to a human-readable "Validation"
section:

- +optional → **Validation:** Optional.
- +default=X → **Validation:** Default: X.
- +kubebuilder:validation:Minimum=X → **Validation:** Minimum: X.
- +kubebuilder:validation:Maximum=X → **Validation:** Maximum: X.
- +kubebuilder:validation:MinLength=X → **Validation:** Minimum length: X.
- +kubebuilder:validation:MaxLength=X → **Validation:** Maximum length: X.
- +kubebuilder:validation:MinItems=X → **Validation:** Minimum items: X.
- +kubebuilder:validation:MaxItems=X → **Validation:** Maximum items: X.
- +kubebuilder:validation:Pattern=X → **Validation:** Pattern: X.
- +kubebuilder:validation:Enum=A;B → **Validation:** Valid values: A, B.

Internal annotations are removed: +patchStrategy, +patchMergeKey, +listType,
+listMapKey, +featureGate, +structType, +protobuf, +union, +enum,
+kubebuilder:validation:XValidation (complex CEL rules).

Signed-off-by: Alan Clucas <[email protected]>
@Joibel Joibel added the area/docs Incorrect, missing, or mistakes in docs label Dec 3, 2025
@Joibel Joibel requested review from Copilot and tico24 December 3, 2025 14:29
@Joibel Joibel marked this pull request as draft December 3, 2025 14:30
@Joibel Joibel marked this pull request as ready for review December 3, 2025 14:30
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a Python script to clean up kubebuilder annotations in the generated executor_swagger.md documentation, making it more readable by converting raw annotations into a structured "Validation" section.

Key Changes:

  • Adds hack/docgen/clean_swagger_md.py to parse and transform kubebuilder annotations
  • Converts annotations like +optional, +kubebuilder:validation:Minimum=X, etc. into human-readable validation descriptions
  • Removes internal annotations (+patchStrategy, +listType, +protobuf, etc.) that are not useful in user-facing documentation

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

File Description
hack/docgen/clean_swagger_md.py New Python script that parses annotations from markdown, converts them to readable validation sections, and removes internal implementation details
docs/executor_swagger.md Generated documentation with cleaned-up annotations - all kubebuilder annotations are now converted to "Validation:" sections with semicolon-separated constraints
Makefile Updated codegen target to use the new Python script instead of the previous sed command that only fixed interface{} links

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Joibel and others added 2 commits December 3, 2025 14:46
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Alan Clucas <[email protected]>
Signed-off-by: Alan Clucas <[email protected]>
@tico24
Copy link
Member

tico24 commented Dec 5, 2025

@CodeRabbit full review

@coderabbitai
Copy link

coderabbitai bot commented Dec 5, 2025

✅ Actions performed

Full review triggered.

@coderabbitai
Copy link

coderabbitai bot commented Dec 5, 2025

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

The Makefile's codegen target replaces a direct sed command with an invocation of a new Python script (hack/docs/clean_swagger_md.py) to process Swagger-generated Markdown. The script parses kubebuilder annotations and converts them into human-readable Validation sections within documentation.

Changes

Cohort / File(s) Summary
Build system update
Makefile
Modified codegen target to invoke Python script instead of inline sed for Swagger Markdown cleanup
Documentation processing utility
hack/docs/clean_swagger_md.py
New Python script that parses kubebuilder annotations from Swagger Markdown and constructs human-readable Validation sections with metadata (optional/required, default values, enum values, numeric constraints, patterns); includes table cell processing and inline annotation removal

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Annotation parsing logic: Review regex patterns and extraction logic for correctness across various kubebuilder annotation formats
  • Markdown processing: Verify table cell handling, line-by-line transformation, and preservation of non-annotation content
  • Edge cases: Validate handling of inline formatting, nested structures, and boundary conditions in the clean_swagger_md.py script
  • Makefile integration: Confirm script path and invocation are correct

Poem

🐰 A script hops in to tidy swagger's sprawl,
Annotations morph to validation—readable for all!
No more kubebuilder clutter in our markdown lane,
Just human-friendly defaults, enums, and constraint refrain! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding cleanup of kubebuilder annotations in executor_swagger.md documentation.
Description check ✅ Passed The description includes Motivation, Modifications with specific annotation conversions, and Verification sections, though it lacks an issue reference and has minimal Documentation detail.
Docstring Coverage ✅ Passed Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (1)
hack/docgen/clean_swagger_md.py (1)

60-64: Numeric validation regex doesn't match negative values.

Same issue as in hack/docs/clean_swagger_md.py - the regex \d+ won't capture negative numbers for Minimum and Maximum constraints. This was flagged in a past review.

     for key, validator in validation_fields:
         text, annotations[key] = extract_and_remove(
             text,
-            rf'\+kubebuilder:validation:{validator}=(\d+)',
-            rf'\+kubebuilder:validation:{validator}=\d+')
+            rf'\+kubebuilder:validation:{validator}=([+-]?\d+)',
+            rf'\+kubebuilder:validation:{validator}=[+-]?\d+')
🧹 Nitpick comments (1)
hack/docs/clean_swagger_md.py (1)

60-64: Inconsistency with hack/docgen/clean_swagger_md.py for numeric validation patterns.

This file uses \d+ for numeric validations, while hack/docgen/clean_swagger_md.py uses [+-]?\d+ which also handles negative values. For Minimum and Maximum constraints, negative values are valid (e.g., temperatures, offsets). Consider aligning with the other script.

     for key, validator in validation_fields:
         text, annotations[key] = extract_and_remove(
             text,
-            rf'\+kubebuilder:validation:{validator}=(\d+)',
-            rf'\+kubebuilder:validation:{validator}=\d+')
+            rf'\+kubebuilder:validation:{validator}=([+-]?\d+)',
+            rf'\+kubebuilder:validation:{validator}=[+-]?\d+')
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a7caf0 and ce85610.

📒 Files selected for processing (3)
  • Makefile (1 hunks)
  • hack/docgen/clean_swagger_md.py (1 hunks)
  • hack/docs/clean_swagger_md.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
hack/docs/clean_swagger_md.py (1)
hack/docgen/clean_swagger_md.py (3)
  • extract_and_remove (12-18)
  • parse_annotations (21-97)
  • build_validation_section (100-130)
🔇 Additional comments (3)
Makefile (1)

356-357: LGTM!

The Makefile correctly invokes the new Python script to clean up executor_swagger.md during code generation. The comment clearly explains the purpose.

hack/docgen/clean_swagger_md.py (1)

215-221: Encoding handling is correct.

The encoding='utf-8' parameter is properly specified for both read and write operations, addressing the concern from the previous review.

hack/docs/clean_swagger_md.py (1)

1-227: Verify existence of hack/docgen/clean_swagger_md.py and compare with hack/docs/clean_swagger_md.py.

The review comment claims duplicate scripts exist with minor differences, but this requires manual verification of whether both files exist in the repository and confirmation of their actual similarities and differences before determining if consolidation is necessary.

Signed-off-by: Alan Clucas <[email protected]>
@Joibel
Copy link
Member Author

Joibel commented Dec 5, 2025

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Dec 5, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/docs Incorrect, missing, or mistakes in docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants