-
Notifications
You must be signed in to change notification settings - Fork 409
feature add chart template and value api #551
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
Signed-off-by: zerohzz <zzhzero@qq.com>
cbuto
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.
thanks for the PR! a few comments below.
Closes #509.
Signed-off-by: zerohzz <zzhzero@qq.com>
|
The PR relates to #509 ? |
cbuto
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.
few small nits around style
Signed-off-by: zerohzz <zzhzero@qq.com>
|
Another question , should this new handler be protected with the auth mechanism? /cc @cbuto |
|
@scbizu we definitely need auth for these endpoints if they aren't already covered 👀 but it looks like it is. |
cbuto
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.
lgtm after @scbizu's concerns are addressed! 🎉
Signed-off-by: zzhzero <zzhzero@qq.com>
|
want to give it another look @scbizu? |
|
LGTM , let me do the re-format things , thank you @zzhzero 🎉 |
No description provided.