-
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
chore: inject imageInfo when expanding templates for ko builder #9207
chore: inject imageInfo when expanding templates for ko builder #9207
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #9207 +/- ##
==========================================
- Coverage 70.48% 63.65% -6.83%
==========================================
Files 515 632 +117
Lines 23150 32554 +9404
==========================================
+ Hits 16317 20722 +4405
- Misses 5776 10237 +4461
- Partials 1057 1595 +538 ☔ View full report in Codecov by Sentry. |
pkg/skaffold/build/ko/builder.go
Outdated
"IMAGE_REPO": ref.Repo, | ||
"IMAGE_NAME": ref.Name, | ||
"IMAGE_TAG": ref.Tag, |
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.
I found a var in https://github.com/GoogleContainerTools/skaffold/blob/main/pkg/skaffold/constants/constants.go#L139, ImageRef
that has 2 out of 3 of these values, do you think is a good idea to use that constant instead? We would need to add IMAGE_NAME
if we decide we want to use it
@@ -150,11 +151,44 @@ func TestBuildOptions(t *testing.T) { | |||
UserAgent: version.UserAgentWithClient(), | |||
}, | |||
}, | |||
{ | |||
description: "", |
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.
Could we add a description for this test case? Just to be able to detect it better in the logs if something happens. Thanks! 😄
1c67fff
to
e4568e4
Compare
Fixes: #9045
Related: Relevant tracking issues, for context
Merge before/after: Dependent or prerequisite PRs
Description