fix: generate manifest in conformance with the deployment spec #530
Conversation
WalkthroughThe changes update the Changes
Sequence Diagram(s)sequenceDiagram
participant Helm Chart
participant Kubernetes API
Helm Chart->>Kubernetes API: Deploy resource
alt If kind == StatefulSet
Helm Chart->>Kubernetes API: Include serviceName and podManagementPolicy in spec
else Other kinds
Helm Chart->>Kubernetes API: Exclude serviceName and podManagementPolicy from spec
end
Estimated code review effort2 (~15 minutes) Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
dud225
force-pushed
the
graph_node_schema
branch
2 times, most recently
from
f5759e9 to
7c30c01
Compare
dud225
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/graph-node/Chart.yaml (1)
25-25: Chart appVersion updated correctly; consider bumping chart version
TheappVersionhas been bumped fromv0.38.0tov0.39.0, which aligns with the new Graph Node release. Per Helm SemVer best practices, please also increment the chart’sversion(currently0.5.9) to0.5.10to reflect this change in the chart artifacts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
charts/graph-node/Chart.yaml(1 hunks)charts/graph-node/README.md(1 hunks)charts/graph-node/templates/graph-node/all.yaml(1 hunks)
🔇 Additional comments (2)
charts/graph-node/README.md (1)
5-5: Update version badge for AppVersion
The README badge forAppVersionhas been updated tov0.39.0, keeping it in sync withChart.yaml.charts/graph-node/templates/graph-node/all.yaml (1)
65-74: Conditionally include serviceName only for StatefulSet
TheserviceName(and accompanying StatefulSet properties) has been moved inside theif (eq $values.kind "StatefulSet")block to prevent schema non-conformance for other kinds (e.g.,Deployment). Verify that indentation underspec:remains correct and that rendering withhelm templateorhelm lintsucceeds for both resource kinds.
|
Hi @dud225 , sorry for the long time to reply. First of all, thank you for your contribution! 🥳 Would you be willing to rebase this to main and reduce it to the bugfix (which I very much would like to get in)? While bumping chart version a patch version up? 🙏 |
dud225
force-pushed
the
graph_node_schema
branch
from
7c30c01 to
eaefb0a
Compare
|
@cjorge-graphops done |
|
Thank you @dud225! for both your contribution and quick response ⭐ , much appreciated. Releasing this soon. |
Summary by CodeRabbit
Chores
Refactor