Conversation
dgageot
force-pushed
the
improve-runner-code
branch
from
9cc6cef to
b3f8ceb
Compare
Codecov Report
@@ Coverage Diff @@
## master #1204 +/- ##
==========================================
+ Coverage 42.24% 42.49% +0.25%
==========================================
Files 96 101 +5
Lines 4242 4292 +50
==========================================
+ Hits 1792 1824 +32
- Misses 2276 2295 +19
+ Partials 174 173 -1
Continue to review full report at Codecov.
|
dgageot
force-pushed
the
improve-runner-code
branch
4 times, most recently
from
7f0d914 to
745be3f
Compare
|
ping @GoogleContainerTools/container-tools can you please take a look. I tried to remove a lot of duplication in the runner code. It was becoming a bit too complicated to maintain. |
r2d4
left a comment
r2d4
left a comment
There was a problem hiding this comment.
Can you split out the error text changes as a new PR?
Signed-off-by: David Gageot <david@gageot.net>
Signed-off-by: David Gageot <david@gageot.net>
Both commands should follow the same path except `deploy` will work with pre-built images. Signed-off-by: David Gageot <david@gageot.net>
Signed-off-by: David Gageot <david@gageot.net>
Signed-off-by: David Gageot <david@gageot.net>
Signed-off-by: David Gageot <david@gageot.net>
Signed-off-by: David Gageot <david@gageot.net>
Signed-off-by: David Gageot <david@gageot.net>
dgageot
force-pushed
the
improve-runner-code
branch
from
745be3f to
b85b7bf
Compare
| imageList := kubernetes.NewImageList() | ||
| for _, b := range bRes { | ||
| imageList.Add(b.Tag) | ||
| if err = r.Test(ctx, out, bRes); err != nil { |
There was a problem hiding this comment.
How about skip if test is nil and get rid of noop tester?
There was a problem hiding this comment.
if tests is nil? Do you mean we'd have to store the SkaffoldPipeline in the SkaffoldRunner instance?
| } | ||
|
|
||
| func (b *prebuiltImagesBuilder) Build(ctx context.Context, out io.Writer, tagger tag.Tagger, artifacts []*latest.Artifact) ([]Artifact, error) { | ||
| tags := make(map[string]string) |
There was a problem hiding this comment.
Since we’re already here, how about we generate the git tags if no images were supplied?
There was a problem hiding this comment.
We can also resolve the taggers from the yams if you want to be really correct (to make sure we can generate reproducibly)
There was a problem hiding this comment.
If images isn't provided, try to seed it by using the passed in tagger to generate a tag for each artifact
if images == nil {
for _, a := range artifacts {
tag, err := tag.GenerateFullyQualifiedTag(a.Image, nil)
if err != nil // this is the case where we ended up with a tagger that needs some options passed in, fail
images = append(images, tag)
}| } | ||
|
|
||
| func getBuilder(cfg *latest.BuildConfig, kubeContext string) (build.Builder, error) { | ||
| func getBuilder(cfg *latest.BuildConfig, kubeContext string, opts *config.SkaffoldOptions) (build.Builder, error) { |
3 participants
Let's remove some duplication and simplify the runner's code.