feat: ensure CRD comments are clean#85
Conversation
The following requirements apply to keep the generated CRDs clean: 1. Default value ends up on its own line. 2. Individual lines are limited to 80 chars each. They are wrapped at 80 chars. 3. Kubebuilder comments are not wrapped so that they may continue to modify CRDs as we see fit. 4. Blocks have spaces between them. Signed-off-by: Scott Lee <scott.lee01818@gmail.com>
|
| Filename | Overview |
|---|---|
| internal/workload/v1/markers/markers.go | Adds five new helpers for clean CRD comment generation; wordRe is correctly package-level; logic is sound |
| internal/workload/v1/markers/markers_internal_test.go | Adds tests for all four user-facing helpers; one test case comment claims '80 characters exactly' but the string is 78 chars |
| internal/workload/v1/markers/field_marker.go | Adds GetComments method delegating to commentsFromMarker; straightforward and correct |
| internal/workload/v1/markers/collection_field_marker.go | Adds GetComments method delegating to commentsFromMarker; mirrors the FieldMarker change correctly |
| internal/workload/v1/kinds/workload.go | Replaces raw strings.Split with marker.GetComments, delegating all formatting to the new helpers |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["marker.GetComments('+kubebuilder:')"] --> B["commentsFromMarker(description, exceptions)"]
B --> C{"description empty after newline trim?"}
C -- yes --> D["return nil"]
C -- no --> E["prepend leading blank"]
E --> F["buildDescriptionBlocks"]
F --> G["paragraph blocks"]
G --> I["for each block"]
I --> J["wrapCommentBlock"]
J --> K{"exception prefix?"}
K -- yes --> L["emit verbatim"]
K -- no --> M["accumulate pending"]
M --> N["wrapCommentLine"]
N --> O{"text over 80 chars?"}
O -- no --> P["single line"]
O -- yes --> Q["split at word boundary"]
L --> R["block result"]
P --> R
Q --> R
R --> S["blank between blocks"]
S --> T["normalizeCommentLines"]
T --> U["comments passed to AddField"]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["marker.GetComments('+kubebuilder:')"] --> B["commentsFromMarker(description, exceptions)"]
B --> C{"description empty after newline trim?"}
C -- yes --> D["return nil"]
C -- no --> E["prepend leading blank"]
E --> F["buildDescriptionBlocks"]
F --> G["paragraph blocks"]
G --> I["for each block"]
I --> J["wrapCommentBlock"]
J --> K{"exception prefix?"}
K -- yes --> L["emit verbatim"]
K -- no --> M["accumulate pending"]
M --> N["wrapCommentLine"]
N --> O{"text over 80 chars?"}
O -- no --> P["single line"]
O -- yes --> Q["split at word boundary"]
L --> R["block result"]
P --> R
Q --> R
R --> S["blank between blocks"]
S --> T["normalizeCommentLines"]
T --> U["comments passed to AddField"]
Reviews (4): Last reviewed commit: "test: add unit tests for new functions" | Re-trigger Greptile
| // wrapCommentLine wraps text at commentWrapWidth characters, breaking only at word | ||
| // boundaries. Whitespace runs between words (e.g. double-spaces after a | ||
| // sentence-ending period) are preserved in the output. | ||
| func wrapCommentLine(text string) []string { | ||
| if text == "" { | ||
| return nil | ||
| } | ||
|
|
||
| wordRe := regexp.MustCompile(`\S+`) | ||
| locs := wordRe.FindAllStringIndex(text, -1) | ||
|
|
||
| if len(locs) == 0 { | ||
| return nil | ||
| } | ||
|
|
||
| var lines []string | ||
|
|
||
| lineStart := locs[0][0] | ||
| lineEnd := locs[0][1] | ||
|
|
||
| for _, loc := range locs[1:] { | ||
| wordEnd := loc[1] | ||
| if wordEnd-lineStart > commentWrapWidth { | ||
| lines = append(lines, text[lineStart:lineEnd]) | ||
| lineStart = loc[0] | ||
| } | ||
|
|
||
| lineEnd = wordEnd | ||
| } | ||
|
|
||
| lines = append(lines, text[lineStart:lineEnd]) | ||
|
|
||
| return lines | ||
| } |
There was a problem hiding this comment.
No tests for new comment-formatting helpers
commentsFromMarker, wrapCommentBlock, wrapCommentLine, and isCommentLineException are non-trivial functions with several edge cases (empty description, all-exception blocks, single words longer than 80 chars, multi-block descriptions, descriptions that start or end with blank lines). The existing markers_internal_test.go has thorough coverage for the other helpers at the same granularity — these four have none. A regression in any wrapping edge case would silently produce malformed CRD comments.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Signed-off-by: Scott Lee <scott.lee01818@gmail.com>
Signed-off-by: Scott Lee <scott.lee01818@gmail.com>
d9f1911 to
01dab68
Compare
The following requirements apply to keep the generated CRDs clean: