-
Notifications
You must be signed in to change notification settings - Fork 428
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
fix: respect image.location
uri for containers
#5555
Conversation
3cdbee2
to
f1e340e
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## mainline #5555 +/- ##
============================================
- Coverage 70.05% 70.05% -0.01%
============================================
Files 302 302
Lines 46335 46338 +3
Branches 301 301
============================================
+ Hits 32459 32460 +1
- Misses 12302 12305 +3
+ Partials 1574 1573 -1 ☔ View full report in Codecov by Sentry. |
🍕 Here are the new binary sizes!
|
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.
Looks good, waiting for tests to be finished to approve 👍
imageURI := rc.PushedImages[name].URI() | ||
var imageURI string | ||
if image, ok := rc.PushedImages[name]; ok { | ||
imageURI = image.URI() | ||
} |
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.
Not feedback but I found this snippet of code to be really interesting in Go!
The new code is definitely a more idiomatic way to approach the problem because it doesn't use what feels like a side effect of the language, but they actually have the exact same output. I was curious so I wrote this in the Go Playground to test.
❤️ Great catch on this since it's not actually part of the bug.
f1e340e
to
5c366ff
Compare
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.
lgtm - e2e tests please pass 🙏
Addresses: #5501 (comment)
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.