Skip to content
This repository was archived by the owner on Jul 18, 2025. It is now read-only.

Fix helm generation#335

Merged
vdemeester merged 1 commit intodocker-archive-public:masterfrom
vdemeester:fix-helm-generation
Aug 2, 2018
Merged

Fix helm generation#335
vdemeester merged 1 commit intodocker-archive-public:masterfrom
vdemeester:fix-helm-generation

Conversation

@vdemeester
Copy link
Copy Markdown
Contributor

@vdemeester vdemeester commented Aug 2, 2018

This bump the docker/cli version to get the fix required on
ExtractVariables. It now takes a pattern other than the default,
which is something we require on docker/app as the pattern we
use (when interpolating) is a bit different from the default one.

It also adds a small message on the e2e test errors to see which file
was wrong.

Depends on docker/cli#1262

Signed-off-by: Vincent Demeester vincent@sbr.pm

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 2, 2018

Codecov Report

Merging #335 into master will decrease coverage by 0.07%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #335      +/-   ##
==========================================
- Coverage   56.13%   56.05%   -0.08%     
==========================================
  Files          50       50              
  Lines        2551     2551              
==========================================
- Hits         1432     1430       -2     
- Misses        913      914       +1     
- Partials      206      207       +1
Impacted Files Coverage Δ
internal/packager/init.go 64.46% <100%> (ø) ⬆️
internal/render/render.go 57.33% <100%> (ø) ⬆️
internal/helm/helm.go 54.5% <66.66%> (-0.82%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 23bdc1e...f33314d. Read the comment docs.

Copy link
Copy Markdown
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

Waiting for docker/cli#1262 to be merged

This bump the docker/cli version to get the fix required on
`ExtractVariables`. It now takes a pattern other than the default,
which is something we require on `docker/app` as the pattern we
use (when interpolating) is a bit different from the default one.

It also adds a small message on the e2e test errors to see which file
was wrong.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
Copy link
Copy Markdown
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread e2e/commands_test.go
golden.Assert(t, string(chart), "helm-expected.chart/Chart.yaml")
golden.Assert(t, string(values), "helm-expected.chart/values.yaml")
golden.Assert(t, string(stack), "helm-expected.chart/templates/stack.yaml")
golden.Assert(t, string(chart), "helm-expected.chart/Chart.yaml", "chart file is wrong")
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.

and you should feel wrong!
"invalid chart file"?

@vdemeester vdemeester merged commit 0b92d64 into docker-archive-public:master Aug 2, 2018
@vdemeester vdemeester deleted the fix-helm-generation branch August 2, 2018 16:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants