-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Allow multiple configs in single skaffold.yaml #5199
Allow multiple configs in single skaffold.yaml #5199
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5199 +/- ##
==========================================
+ Coverage 71.82% 71.87% +0.04%
==========================================
Files 387 387
Lines 13928 13974 +46
==========================================
+ Hits 10004 10044 +40
- Misses 3190 3193 +3
- Partials 734 737 +3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thinking about this a little more, i don't know that we want to require all configs passed in to explicitly have the same API version.
in most cases, we should be able to take the latest found API version, and skaffold fix
up the older ones to that new version successfully. this won't always work (in the event that there was a breaking config change at some point), but these are usually rare so it should work in the majority of cases. WDYT?
So this only limits all configs defined in the same |
ahh ok, sorry i didn't follow that. i don't think it's a huge deal, but if different files can have different config versions, and then i can combine those separate files into a single (legal) skaffold.yaml which now will fail because the config versions are different, that feels a bit arbitrary no? i won't die on this hill because it's pretty unconventional and might make the implementation more annoying for you since you'll have to figure out how to make multi-config skaffold.yamls work with |
You're right. It now supports multiple config versions. It was mostly an arbitrary choice on my part and not implementation difficulty. Fixed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM 👍🏼
Related: Skaffold Multiple Configs support
This PR implements defining multiple skaffold pipelines in a single file. Maintaining multiple configurations is already implemented in #5160