Skip to content

feature add chart template and value api#551

Merged
scbizu merged 6 commits into
helm:mainfrom
zzhzero:main
Apr 7, 2022
Merged

feature add chart template and value api#551
scbizu merged 6 commits into
helm:mainfrom
zzhzero:main

Conversation

@zzhzero

@zzhzero zzhzero commented Feb 11, 2022

Copy link
Copy Markdown
Contributor

No description provided.

@cbuto cbuto left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread pkg/chartmuseum/server/multitenant/handlers.go
Comment thread pkg/chartmuseum/server/multitenant/handlers.go Outdated
@helm-bot helm-bot added size/L and removed size/M labels Feb 12, 2022
@scbizu

scbizu commented Feb 13, 2022

Copy link
Copy Markdown
Contributor

The PR relates to #509 ?

@cbuto cbuto left a comment

Copy link
Copy Markdown
Contributor

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

Comment thread pkg/chartmuseum/server/multitenant/server_test.go Outdated
Comment thread pkg/chartmuseum/server/multitenant/server_test.go Outdated
Comment thread pkg/chartmuseum/server/multitenant/server_test.go Outdated
Comment thread pkg/chartmuseum/server/multitenant/server_test.go Outdated
Comment thread pkg/chartmuseum/server/multitenant/api.go
Comment thread pkg/chartmuseum/server/multitenant/api.go Outdated
Comment thread pkg/chartmuseum/server/multitenant/api.go Outdated
@cbuto cbuto added this to the v0.15.0 milestone Feb 15, 2022
Comment thread pkg/chartmuseum/server/multitenant/handlers.go
Comment thread pkg/chartmuseum/server/multitenant/handlers.go
@scbizu

scbizu commented Feb 23, 2022

Copy link
Copy Markdown
Contributor

Another question , should this new handler be protected with the auth mechanism?

/cc @cbuto

@cbuto

cbuto commented Mar 4, 2022

Copy link
Copy Markdown
Contributor

@scbizu we definitely need auth for these endpoints if they aren't already covered 👀 but it looks like it is.

@cbuto cbuto left a comment

Copy link
Copy Markdown
Contributor

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! 🎉

@cbuto

cbuto commented Apr 6, 2022

Copy link
Copy Markdown
Contributor

want to give it another look @scbizu?

@scbizu
scbizu merged commit 315ddf9 into helm:main Apr 7, 2022
@scbizu

scbizu commented Apr 7, 2022

Copy link
Copy Markdown
Contributor

LGTM , let me do the re-format things , thank you @zzhzero 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants