-
Notifications
You must be signed in to change notification settings - Fork 3.5k
docs: filter metadata labels from fields.md #15141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Alan Clucas <[email protected]>
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThe documentation generation logic is refined to introduce cross-field indexing for "MetricLabel" types and implement selective example rendering based on field type and jsonName. Concurrently, documentation content is consolidated by replacing verbose example lists with compact summary headers. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/fields.md (1)
1726-1726: Fix typos: “metdata” and possessive.Two minor docs typos.
-## Metadata -Pod metdata +## Metadata +Pod metadata-|`metadata`|[`Metadata`](#metadata)|Metdata sets the pods's metadata, i.e. annotations and labels| +|`metadata`|[`Metadata`](#metadata)|Metadata sets the pod's metadata, i.e. annotations and labels|Also applies to: 1844-1844
🧹 Nitpick comments (2)
docs/fields.md (1)
489-489: Standardize “Examples” summary phrasing across sections.Some sections use “Examples with this field (click to open)” while others use “Examples (click to open)”. Pick one for consistency (site-wide find/replace in generator).
Also applies to: 955-955, 1411-1411, 1693-1693, 1710-1711, 3978-3978
hack/docs/fields.go (1)
265-277: The cross-indexing logic for MetricLabel is sound.The approach of identifying MetricLabel-relevant files by requiring both "prometheus:" and "labels:" patterns effectively narrows down to prometheus metrics configuration examples.
Minor observation: the map-initialization pattern (lines 270-275) is repeated three times in this function (also at lines 247-252 and 258-263). Consider extracting a helper like
addToIndex(indexName, fileName string)to reduce duplication, though this is optional.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/fields.md(1 hunks)hack/docs/fields.go(2 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: Codegen
- GitHub Check: argo-images (argoexec)
- GitHub Check: argo-images (argocli)
🔇 Additional comments (2)
docs/fields.md (1)
3978-3983: MetricLabel examples filtered correctly; verify generator gating logic.The examples section at lines 3978-3983 displays the expected two files (custom-metrics.yaml and dag-custom-metrics.yaml) with correct formatting. The change aligns with the stated objective to filter labels appropriately for MetricLabel.
However, the verification of the generator gating logic in hack/docs/fields.go could not be completed due to sandbox limitations. The original reviewer's assessment of the examples themselves appears sound based on the visible code snippet.
hack/docs/fields.go (1)
320-331: Filtering logic correctly scopes examples to relevant field types.The two-tier approach works well:
- MetricLabel (and similar types) receive examples via the type-name lookup at lines 313-315, using the new cross-index.
- The jsonName-based lookup is appropriately suppressed for "labels" when
name != "ObjectMeta", preventing the broad "labels:" matches from polluting the MetricLabel documentation.This aligns with the PR objective of filtering out incorrect fields from the MetricLabel section.
Signed-off-by: Alan Clucas <[email protected]>
Signed-off-by: Alan Clucas <[email protected]>
Signed-off-by: Alan Clucas <[email protected]>
Motivation
In #15063 we've added loads more metadata labels to examples. This highlights that
fields.mdis picking up too many incorrect fields.Modifications
Modified
fields.goto do magic filtering to distinguish both types of label.Verification
Rebuilt fields.md and cross referenced resulting links which look correct.
Documentation
Yes, this is. Thanks.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.