-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
feat: Go template with sprig for application template render in applicationset #9873
Conversation
…cationset Signed-off-by: rishabh625 <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #9873 +/- ##
==========================================
- Coverage 45.86% 45.69% -0.18%
==========================================
Files 227 228 +1
Lines 26864 26932 +68
==========================================
- Hits 12322 12307 -15
- Misses 12862 12949 +87
+ Partials 1680 1676 -4
Continue to review full report at Codecov.
|
Signed-off-by: rishabh625 <[email protected]>
8958ac2
to
fd18257
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.
@rishabh625 can you write some usage docs? Having a bit of trouble following the code, and I think that'll help.
if applicationSetTemplate != nil { | ||
err = mergo.Merge(dest, applicationSetTemplate) | ||
} else { | ||
log.Warn("generator template won't be applied when standard application template is not used") |
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.
Is this something we can't support? Seems that we could 1) render the untypedTemplate, 2) unmarshal the rendered template, 3) merge it with the generator template and 4) render the merged template.
@@ -13,266 +13,6 @@ import ( | |||
argoappsetv1 "github.com/argoproj/argo-cd/v2/pkg/apis/applicationset/v1alpha1" | |||
) | |||
|
|||
func TestRenderTemplateParams(t *testing.T) { |
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.
No replacement for these tests?
@iamnoah Is this PR still worked on ? |
@crenshaw-dev I would be happy to try to push something but it might be sloppy attempt. As I have a limited experience with Golang. Would that be good for you ? |
@mrmm I'd be happy to review and help with any local dev setup issues! :-) |
Closing for now, until someone has time to pick it back up. :-) |
Hello @crenshaw-dev, I hope you are doing great!
Edit: @crenshaw-dev you can ignore my previous request, as the work on this has restarted in #11183 |
Signed-off-by: rishabh625 [email protected]
Note on DCO:
If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.
Checklist:
This is the PR moved from argoproj/applicationset
argoproj/applicationset#513
Thanks to @vavdoshka for all the efforts and @crenshaw-dev for pushing it this close