Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Push once #2855

Merged
merged 1 commit into from
Oct 2, 2019
Merged

Push once #2855

merged 1 commit into from
Oct 2, 2019

Conversation

dgageot
Copy link
Contributor

@dgageot dgageot commented Sep 11, 2019

Image that were already pushed don't need to be pushed again

@dgageot
Copy link
Contributor Author

dgageot commented Sep 11, 2019

Tests pass on my machine..

@dgageot dgageot force-pushed the push-once branch 2 times, most recently from ad8d1d1 to eb598a2 Compare September 11, 2019 15:30
@codecov
Copy link

codecov bot commented Sep 11, 2019 •
edited
Loading

Codecov Report

Merging #2855 into master will decrease coverage by 0.02%.
The diff coverage is 82.35%.

Impacted Files Coverage Δ
pkg/skaffold/docker/image.go 72.81% <82.35%> (+0.81%) ⬆️
pkg/skaffold/docker/auth.go 62.79% <0%> (-9.31%) ⬇️

@dgageot dgageot mentioned this pull request Sep 11, 2019
Closed
nkubala
nkubala reviewed Sep 13, 2019
View reviewed changes
Copy link
Contributor

@nkubala nkubala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one comment but otherwise LGTM

pkg/skaffold/docker/image.go Outdated Show resolved Hide resolved
tejal29
tejal29 suggested changes Sep 13, 2019
View reviewed changes
pkg/skaffold/docker/image.go Outdated Show resolved Hide resolved
pkg/skaffold/docker/image.go Outdated Show resolved Hide resolved
pkg/skaffold/docker/image_test.go Show resolved Hide resolved
@dgageot
Copy link
Contributor Author

dgageot commented Sep 30, 2019

@tejal29 @nkubala this should be better now

@dgageot dgageot merged commit 8e42805 into GoogleContainerTools:master Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nkubala nkubala nkubala left review comments

@tejal29 tejal29 tejal29 approved these changes

@balopat balopat Awaiting requested review from balopat

@priyawadhwa priyawadhwa Awaiting requested review from priyawadhwa

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dgageot @tejal29 @nkubala @googlebot