Skip to content
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

Helm chart #70

Merged
merged 4 commits into from
Jan 8, 2020
Merged

Helm chart #70

merged 4 commits into from
Jan 8, 2020

Conversation

marcostvz
Copy link
Contributor

Helm chart proposal, to solve #69

Let me know if you prefer it in a different folder.

@pusher-ci
Copy link

Hi @marcostvz. Thanks for your PR.

I'm waiting for a pusher member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link
Collaborator

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know very much about helm as we don't use it here as Pusher, but happy to have a chart and what I see here seems sensible given my little helm knowledge 🙂

Out of interest, do you think this is something we could propose to the upstream charts repository rather than keeping it here? It was my understanding that the upstream charts repo is the goto for charts?

@JoelSpeed
Copy link
Collaborator

/ok-to-test

@marcostvz
Copy link
Contributor Author

do you think this is something we could propose to the upstream charts repository rather than keeping it here?

I believe it makes sense, yes. :) It will also give visibility to your project.

@JoelSpeed
Copy link
Collaborator

@marcostvz Since you've come up with this chart, would you feel comfortable trying to add it to https://github.com/helm/charts? I feel it would be unfair for us to copy your work over

@marcostvz
Copy link
Contributor Author

Hey @JoelSpeed , I added a PR into the helm/incubator, I thought it made more sense as I believe it needs more testing before going into helm/stable.

helm/charts#17866

@marcostvz
Copy link
Contributor Author

marcostvz commented Oct 10, 2019

Also, please note that I added you and I as approvers/reviewers. If you want to remove me, glad to not be there haha. Anyway tell me if you want more people to be added.
https://github.com/helm/charts/pull/17866/files#diff-8b411be2d6d255adb8adacb57c2c949b

@JoelSpeed
Copy link
Collaborator

@marcostvz That sounds fantastic, thank you so much for your contribution. Let's see how the other PR goes and come back to this once there is a decision

@marcostvz
Copy link
Contributor Author

Hey @JoelSpeed , despite the PR is alright, the people from helm charts do not want to host new charts and are encouraging to move to helm hub. Before doing so, this chart should we available in some repository, they suggest using GitHub pages, but at the moment I do not have time to dedicate to move that forward.

@wonderhoss
Copy link
Collaborator

/retest

@wonderhoss
Copy link
Collaborator

/test 1.12

Copy link
Collaborator

@wonderhoss wonderhoss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wonderhoss wonderhoss merged commit 8bddd32 into wave-k8s:master Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants