-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(helm): DRY cloudsql-proxy #13369
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
|
@fernandezcuesta, could you please check it? |
075a61b to
d73a51b
Compare
mtesauro
left a 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.
Approved
|
@kiblik I will leave to you to merge in case you would want @fernandezcuesta to look closer before it goes in |
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.
LGTM
Little bit weird that the function returns a list rather than a map, something like:
- {{- include "foo" . | nindent 6 }}Where you can visually see it's a single container.
But I feel it's just my own personal bias. Other than that it does its job!
|
Based on the feedback above ☝️ I'm going to go ahead and merge this. |
cloudsql-proxyis always the same. Let's DRY (Don't repeat yourself) it.