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.

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

SyncMap is a matter of artifact type, not builder #3450

Merged
merged 1 commit into from
Jan 8, 2020

Conversation

dgageot
Copy link
Contributor

@dgageot dgageot commented Dec 26, 2019

Fixes #3448

@codecov
Copy link

codecov bot commented Dec 26, 2019

Codecov Report

Merging #3450 into master will increase coverage by 0.4%.
The diff coverage is 77.77%.

Impacted Files Coverage Δ
pkg/skaffold/sync/types.go 100% <ø> (ø) ⬆️
pkg/skaffold/build/build.go 0% <ø> (ø) ⬆️
pkg/skaffold/build/local/local.go 60% <ø> (+5.45%) ⬆️
pkg/skaffold/build/gcb/types.go 31.25% <ø> (+3.47%) ⬆️
pkg/skaffold/build/cluster/types.go 100% <ø> (ø) ⬆️
pkg/skaffold/runner/diagnose.go 12.5% <0%> (ø) ⬆️
pkg/skaffold/sync/sync.go 77.61% <100%> (+1.42%) ⬆️
pkg/skaffold/docker/syncmap.go 67.16% <100%> (ø) ⬆️
pkg/skaffold/runner/dev.go 67.28% <100%> (+0.62%) ⬆️
pkg/skaffold/docker/image.go 77.27% <0%> (+0.9%) ⬆️
... and 4 more

@briandealwis
Copy link
Member

I had been hoping to use this SyncMap() function for #2211 to help the IDEs to identify the application source roots in both the local workspace and remote images. Looking through this PR, I don't think this would have worked 😭. I wish there was a way to avoid these big switches though.

@nkubala
Copy link
Contributor

nkubala commented Jan 8, 2020

@briandealwis ok with the new function name?

Copy link
Member

@briandealwis briandealwis left a comment

Choose a reason for hiding this comment

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

Oops, sorry — didn't mean to block it. LGTM.

@nkubala nkubala merged commit bda1c7f into GoogleContainerTools:master Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SyncMap is not supported by this builder - Using Kaniko and Kustomize
4 participants