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

Automatically fix old configs by default #1259

Merged
merged 7 commits into from
Nov 14, 2018

Conversation

nkubala
Copy link
Contributor

@nkubala nkubala commented Nov 8, 2018

This adds a hook in the runner to automatically try and upgrade older configs to the latest version before running skaffold. This means users with older configs will no longer be required to run skaffold fix to run skaffold as we upgrade the config.

I'll leave skaffold fix in, in case anybody wants to use it to upgrade their config and overwrite it in their repo.

Fixes #1257

pkg/skaffold/schema/versions.go Outdated Show resolved Hide resolved
pkg/skaffold/schema/versions.go Outdated Show resolved Hide resolved
@balopat balopat added this to the Skaffold Beta milestone Nov 9, 2018
Copy link
Contributor

@balopat balopat left a comment

Choose a reason for hiding this comment

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

With this change I would like to cover all the upgrades with unit tests and hunt down if there are any issues.

pkg/skaffold/schema/versions.go Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Nov 13, 2018

Codecov Report

Merging #1259 into master will increase coverage by 1.76%.
The diff coverage is 5.26%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1259      +/-   ##
==========================================
+ Coverage   42.42%   44.19%   +1.76%     
==========================================
  Files          96      104       +8     
  Lines        4247     4614     +367     
==========================================
+ Hits         1802     2039     +237     
- Misses       2269     2366      +97     
- Partials      176      209      +33
Impacted Files Coverage Δ
cmd/skaffold/app/cmd/deploy.go 0% <0%> (ø) ⬆️
cmd/skaffold/app/cmd/run.go 0% <0%> (ø) ⬆️
pkg/skaffold/schema/versions.go 47.22% <0%> (-17.65%) ⬇️
cmd/skaffold/app/cmd/dev.go 0% <0%> (ø) ⬆️
cmd/skaffold/app/cmd/diagnose.go 0% <0%> (ø) ⬆️
cmd/skaffold/app/cmd/runner.go 0% <0%> (ø) ⬆️
cmd/skaffold/app/cmd/delete.go 0% <0%> (ø) ⬆️
cmd/skaffold/app/cmd/build.go 0% <0%> (ø) ⬆️
cmd/skaffold/app/cmd/fix.go 19.35% <100%> (ø) ⬆️
pkg/skaffold/schema/v1alpha2/upgrade.go 68.88% <0%> (ø)
... and 8 more

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 3465168...558ac9f. Read the comment docs.

@nkubala nkubala dismissed balopat’s stale review November 14, 2018 00:11

Added unit tests for all config upgrades

Copy link
Contributor

@balopat balopat left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@nkubala nkubala merged commit 6eae857 into GoogleContainerTools:master Nov 14, 2018
@nkubala nkubala deleted the fix_in_mem branch November 14, 2018 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support old config versions automatically besides skaffold fix
4 participants