-
Notifications
You must be signed in to change notification settings - Fork 22
docs(development): add Helm chart development principles #418
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for cozystack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughA duplicate "Helm Chart Development Principles" section was added to the development documentation file. The new section mirrors the existing one with the same principles and embedded info alerts, positioned before the "Buildx configuration" section. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
Summary of ChangesHello @kvaps, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the project's documentation by introducing a comprehensive guide to Helm chart development principles within Cozystack. It outlines foundational practices for managing upstream charts, ensuring local artifact reproducibility, facilitating efficient local development, and minimizing external dependencies, thereby improving developer understanding and consistency across the project. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request adds a new section to the developer guide detailing Helm chart development principles. The new content is well-structured and clearly explains the core principles. I've identified a small area for improvement regarding redundant information, which can be removed to enhance clarity.
| {{% alert color="info" %}} | ||
| As noted above, full image self-containment is a work in progress. Some packages still pull images from external registries — this is a known gap that we plan to close as capacity allows. | ||
| {{% /alert %}} | ||
|
|
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.
Document the core principles guiding Helm chart packaging in Cozystack: easy upstream updates, local-first artifacts, local dev/test workflow, and no external dependencies. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
02f53a9 to
a59df17
Compare
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: 1
🤖 Fix all issues with AI agents
In `@content/en/docs/v1/development.md`:
- Around line 171-197: Consolidate the two duplicate info alerts (the two {{%
alert color="info" %}} ... {{% /alert %}} blocks) under the "Helm Chart
Development Principles" section into a single alert that contains the combined
message about some packages still referencing externally-built images and the
plan to fully local-build images; remove the second alert block and keep one
clear alert placed near the "Local-first artifacts" or "No external
dependencies" paragraphs so the information remains visible and non-redundant.
🧹 Nitpick comments (1)
content/en/docs/v1/development.md (1)
183-197: Remove redundant info alert.The section contains two info alerts (lines 183-185 and lines 195-197) that communicate essentially the same message about incomplete image self-containment. The second alert (lines 195-197) even explicitly states "As noted above," referencing the first one. This creates unnecessary redundancy.
♻️ Consolidate into a single alert
Consider keeping only the first alert and removing the second, or merge them into a single, more comprehensive note:
### No external dependencies Packages must not depend on external resources at runtime. All charts, images, and patches are vendored into the repository. This guarantees that builds and deployments are deterministic and do not break due to upstream registry outages, removed tags, or network issues. -{{% alert color="info" %}} -As noted above, full image self-containment is a work in progress. Some packages still pull images from external registries — this is a known gap that we plan to close as capacity allows. -{{% /alert %}}The earlier alert at lines 183-185 already covers this concern adequately.
| ## Helm Chart Development Principles | ||
|
|
||
| The package structure and development workflow in Cozystack are guided by the following principles: | ||
|
|
||
| ### Easy to update upstream charts | ||
|
|
||
| The original upstream chart must be easy to update, override, and modify. We use the umbrella chart pattern — upstream charts live in the `./charts` directory and are vendored as-is. Customizations go into `values.yaml` overrides and additional `templates/`, while structural changes to the upstream chart are applied via `patches/`. This separation ensures that updating to a new upstream version is straightforward: run `make update`, review the diff, and re-apply patches if needed. | ||
|
|
||
| ### Local-first artifacts | ||
|
|
||
| Patches and container images are stored locally and are part of the package. The `patches/` directory holds any modifications to the upstream chart, and the `images/` directory contains Dockerfiles for building all required images. This ensures full reproducibility — everything needed to build and deploy the package is self-contained within the repository. | ||
|
|
||
| {{% alert color="info" %}} | ||
| Currently, not all packages build their images locally — some still reference externally-built images. We are actively working toward fully local image builds to achieve complete self-containment and reproducibility. | ||
| {{% /alert %}} | ||
|
|
||
| ### Local development and testing workflow | ||
|
|
||
| Every package must be easy to update and test locally against a real cluster, without relying on CI. The standard `make` targets (`make image`, `make diff`, `make apply`) provide a fast feedback loop: build images, compare rendered manifests against the live cluster, and apply changes — all from a developer's workstation. | ||
|
|
||
| ### No external dependencies | ||
|
|
||
| Packages must not depend on external resources at runtime. All charts, images, and patches are vendored into the repository. This guarantees that builds and deployments are deterministic and do not break due to upstream registry outages, removed tags, or network issues. | ||
|
|
||
| {{% alert color="info" %}} | ||
| As noted above, full image self-containment is a work in progress. Some packages still pull images from external registries — this is a known gap that we plan to close as capacity allows. | ||
| {{% /alert %}} |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for all occurrences of "Helm Chart Development Principles" in the file
rg -n "##\s+Helm Chart Development Principles" content/en/docs/v1/development.md
# Also search for similar headings that might indicate duplication
rg -n -C3 "Helm Chart Development" content/en/docs/v1/development.md
# Get total line count to understand file size
wc -l content/en/docs/v1/development.mdRepository: cozystack/website
Length of output: 418
Remove duplicate alert — the two info boxes convey redundant information.
The section "Helm Chart Development Principles" appears only once in the file (not duplicated). However, the two info alerts within this section (lines 183-185 and 195-197) both communicate the same message: that some packages still reference external images rather than building locally. Consolidate these into a single alert to improve clarity and avoid reader confusion.
🤖 Prompt for AI Agents
In `@content/en/docs/v1/development.md` around lines 171 - 197, Consolidate the
two duplicate info alerts (the two {{% alert color="info" %}} ... {{% /alert %}}
blocks) under the "Helm Chart Development Principles" section into a single
alert that contains the combined message about some packages still referencing
externally-built images and the plan to fully local-build images; remove the
second alert block and keep one clear alert placed near the "Local-first
artifacts" or "No external dependencies" paragraphs so the information remains
visible and non-redundant.
Summary
Test plan
hugo serveSummary by CodeRabbit