Skip to content
This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

httpbin covert to helm chart #965

Merged
merged 2 commits into from
Sep 15, 2020
Merged

httpbin covert to helm chart #965

merged 2 commits into from
Sep 15, 2020

Conversation

knrt10
Copy link
Member

@knrt10 knrt10 commented Sep 15, 2020

closes: #957

Signed-off-by: knrt10 kautilya@kinvolk.io

@knrt10 knrt10 changed the title httpbin covert to helm httpbin covert to helm chart Sep 15, 2020
invidian
invidian reviewed Sep 15, 2020
View reviewed changes
Copy link
Member

@invidian invidian left a comment

Choose a reason for hiding this comment

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

LGTM except the CI failing.

@knrt10 knrt10 force-pushed the knrt10/httpbin-covert-to-helm branch from eeb2f56 to 1422745 Compare September 15, 2020 06:14
@knrt10
Copy link
Member Author

knrt10 commented Sep 15, 2020

I don't know why it's failing, I ran make update-assets. still it fails

@invidian
Copy link
Member

I don't know why it's failing, I ran make update-assets. still it fails

you need to remove empty directory from assets, as update-assets includes it locally, but when you take your branch and do a fresh clone, git won't include empty directories.

@knrt10
httpbin: convert to Helm chart
c39bcca 
closes: #957

Signed-off-by: knrt10 <kautilya@kinvolk.io>
@knrt10
httpbin: add test for manifest rendering
dacd9cc 
Signed-off-by: knrt10 <kautilya@kinvolk.io>
@knrt10 knrt10 force-pushed the knrt10/httpbin-covert-to-helm branch from 1422745 to dacd9cc Compare September 15, 2020 06:23
@knrt10
Copy link
Member Author

knrt10 commented Sep 15, 2020

Thanks, didn't knew about this

@invidian
Copy link
Member

Thanks, didn't knew about this

Hmm, I guess we could run find assets/ -type d -empty | wc -l or something as part of the update-assets and fail if there are empty directories laying around there.

surajssd
surajssd approved these changes Sep 15, 2020
View reviewed changes
Copy link
Member

@surajssd surajssd left a comment

Choose a reason for hiding this comment

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

code looks good to me

@knrt10 knrt10 merged commit b3583fc into master Sep 15, 2020
@knrt10 knrt10 deleted the knrt10/httpbin-covert-to-helm branch September 15, 2020 07:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@surajssd surajssd surajssd approved these changes

@invidian invidian invidian approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

httpbin: convert to helm chart
3 participants
@knrt10 @invidian @surajssd