Introduce config version v1beta2#1376
Merged
balopat merged 5 commits intoGoogleContainerTools:masterfrom Dec 8, 2018
Merged
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1376 +/- ##
=========================================
- Coverage 44.91% 44.42% -0.5%
=========================================
Files 114 109 -5
Lines 4909 4540 -369
=========================================
- Hits 2205 2017 -188
+ Misses 2481 2317 -164
+ Partials 223 206 -17
Continue to review full report at Codecov.
|
dgageot
force-pushed
the
config-v1beta2
branch
from
0038d5c to
95f9aa3
Compare
balopat
suggested changes
Dec 8, 2018
Contributor
balopat
left a comment
balopat
left a comment
There was a problem hiding this comment.
Review checklist:
- diff between
exampleswithintegration/examplesshould be version changes v1beta1 vs v1beta2 for all nontest-(integration test only) examples- there are small whitespace differences between bazel/WORKSPACE - but irrelevant
- manual check that compiled thing works:
examples/getting-started ../../out/skaffold dev Sat Dec 8 12:21:24 2018
WARN[0000] config version (skaffold/v1beta1) out of date: upgrading to latest (skaffold/v1beta2)
- correct upgrade tests - found a copy paste error - v1alpha5 instead of v1beta1, comment added
- upgrade documentation - in v1beta1/upgrade.go I'd like to keep the list of changes as a good practice - even if there are no changes yet - comment added
| ) | ||
|
|
||
| func TestUpgrade(t *testing.T) { | ||
| yaml := `apiVersion: skaffold/v1alpha5 |
Contributor
There was a problem hiding this comment.
Suggested change
| yaml := `apiVersion: skaffold/v1alpha5 | |
| yaml := `apiVersion: skaffold/v1beta1 |
| c.setDefaultKustomizePath() | ||
| c.setDefaultKubectlManifests() | ||
| // Set makes sure default values are set on a SkaffoldPipeline. | ||
| func Set(c *latest.SkaffoldPipeline) error { |
Contributor
There was a problem hiding this comment.
this is elegant, I like the defaults.Set form. Thank you!
| "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util" | ||
| ) | ||
|
|
||
| // Upgrade upgrades a configuration to the next version. |
Contributor
There was a problem hiding this comment.
Suggested change
| // Upgrade upgrades a configuration to the next version. | |
| // Upgrade upgrades a configuration to the next version. | |
| // Config changes from v1beta1 to v1beta2 | |
| // 1. No additions | |
| // 2. No removals | |
| // 3. No updates |
Fixes GoogleContainerTools#1373 Signed-off-by: David Gageot <david@gageot.net>
Signed-off-by: David Gageot <david@gageot.net>
Signed-off-by: David Gageot <david@gageot.net>
+ Removes a lot of dead code + Make it easier to create new config versions Signed-off-by: David Gageot <david@gageot.net>
dgageot
force-pushed
the
config-v1beta2
branch
from
8c7d89c to
6205d10
Compare
Contributor
Author
|
@balopat Thanks for reviewing. It's still cumbersome to introduce a new version, no matter how hard I try to reduced the size of the changeset. With this PR, it should be a bit easier though. |
Contributor
|
Agreed - I think that some of the new version introduction should be scriptable, we can scaffold the basics based on an example of the current and the new config. |
balopat
approved these changes
Dec 8, 2018
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1373
Signed-off-by: David Gageot david@gageot.net