Implement pflag slice value interface for image types #5575
Conversation
Co-authored-by: Marlon Gamez <marlongamez@google.com>
There was a problem hiding this comment.
Skaffold has special defaults handling to workaround spf13/pflag#257 and it currently doesn't support SliceValue, just Value. I was banging on this recently but I haven't gotten a fix in yet. Let me see if I can whip it into shape.
Co-authored-by: Brian de Alwis <bsd@acm.org>
Sure, let me know if you need any help, till then I can help out on some other tickets 👍 |
There was a problem hiding this comment.
@sladyn98 I believe this should be unblocked now and can be finished up soon!
|
@sladyn98 hope you don't mind but I merged this with HEAD (which now has the fixes for SliceValue) and fixed up your implementation: SliceValue objects must implement Value too. |
briandealwis
added
kokoro:run
Codecov Report
@@ Coverage Diff @@
## master #5575 +/- ##
==========================================
- Coverage 70.63% 70.63% -0.01%
==========================================
Files 411 411
Lines 15777 15789 +12
==========================================
+ Hits 11144 11152 +8
- Misses 3812 3817 +5
+ Partials 821 820 -1
Continue to review full report at Codecov.
|
cmd/skaffold/app/flags/image.go
Outdated
| // Set Implements Set() method for pflag interface | ||
| func (p *Images) Set(csv string) error { | ||
| split := strings.Split(csv, ",") | ||
| return p.Replace(split) |
There was a problem hiding this comment.
I realized that this should be .Append() to maintain compatibility with our previous behaviour where we required used to provide multiple -i flags, one for for each image
|
@briandealwis Hey no worries, let me know if these needs any more action, if not I can pick some more issues to solve in the repo |
Fixes: #5479
Description
This one fixes #5479
User facing changes (remove if N/A)