Conversation
|
Got oneOf working and integrated into all our steps. |
nkubala
left a comment
nkubala
left a comment
There was a problem hiding this comment.
Nice! This would definitely be useful outside of skaffold, we should consider moving this code somewhere more central so we can reuse it in other projects.
| val.SetInt(i) | ||
| case reflect.String: | ||
| val.SetString(dt.dv) | ||
| } |
There was a problem hiding this comment.
Is it possible that another type could get passed here? Maybe put a default case here that returns an error so it's more clear that other types aren't supported?
|
@dlorenc Is this PR still active? |
|
cc @nkubala I rebased this and it should be ready to go if we still want it. |
Codecov Report
@@ Coverage Diff @@
## master #388 +/- ##
==========================================
+ Coverage 39.38% 41.17% +1.79%
==========================================
Files 61 62 +1
Lines 2613 2703 +90
==========================================
+ Hits 1029 1113 +84
- Misses 1471 1473 +2
- Partials 113 117 +4
Continue to review full report at Codecov.
|
nkubala
left a comment
nkubala
left a comment
There was a problem hiding this comment.
one small comment, otherwise LGTM
|
|
||
| # Tag the image with the checksum of the built image (image id). | ||
| sha256: {} | ||
| # sha256: {} |
There was a problem hiding this comment.
did you mean to comment this out?
There was a problem hiding this comment.
Yeah, this becomes invalid now. You can't define two tag policies with oneOf.
There was a problem hiding this comment.
Oh sorry, missed the gitCommit tagger above it.
5 participants
This PR adds some new tags we can place on YAML structs that will make management of our config easier.
It adds a 'oneOf' tag, that can be placed on a set of fields and will ensure that only one of them is provided by a user.
It adds a 'required' tag, that can ensure the field has been set by the user.
It adds a 'default' tag, that can be used for strings and ints right now.
Getting this out early for comment. I'll keep cleaning it up and try integrating it into our config parsing next.