-
Notifications
You must be signed in to change notification settings - Fork 5
feat: experiment with Helm Chart Development Workflows using Terraform + Dagger + justfile #45
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
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.
Where's the documentation? I expected to at least find /applications/n8n/README.md
to tell us what the intention is. Introduce us to the workflow and decisions made. There are 117 files in the PR and no entry point or guidance what it's doing. That needs to be correct for anyone to provide a meaningful review. I haven't watched the loom yet and will attempt to do so but a loom does not replace the need for clear documentation.
Lacking that documentation, it appears to me that this pattern is forking upstream charts by directly copying them into the repository? I personally do not like that pattern if that's what I'm seeing.
- This effectively forks any upstream chart with no indication of the version being used. Unless there's workflow here I'm not understanding yet.
- Changes in upstream charts clutter the PRs in this repository possibly pulling in 100's of lines of changes that are just syncing with upstream and not meant for review.
- Look at the deep nesting and stuttering this is causing:
applications/n8n/charts/n8n/charts/valkey/charts/common/Chart.yaml
This file is under the 'charts' folder 3x times.
I put the document here platform-examples/applications/n8n/docs/workflow.md Lines 1 to 5 in b6bf485
I also removed the As explained in the standup, this pattern is an experiment of configuring existing chart with multiple external dependencies. |
} | ||
|
||
const ( | ||
REPLICATED_APP = "gerard-n8n" |
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.
It seems REPLICATED_APP has been hardcode, should we make it dynamic and passed by environment.
@@ -0,0 +1,60 @@ | |||
# This file is maintained automatically by "terraform init". |
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.
I think we can delete this file for now. Since terraform init will generate a new one for it
This PR contains an experiment of configuring/release a chart locally with Terraform + Dagger + justfile.