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

[refactor] make DoInit() a proper controller #3682

Merged

Conversation

nkubala
Copy link
Contributor

@nkubala nkubala commented Feb 12, 2020

last piece of the init refactor. this transfer control of the individual pieces of the init flow to their respective components, and refactors the DoInit() entrypoint to simply control these components without worrying about their implementations.

this PR has no functional change.

@nkubala nkubala added refactor A PR that contains a refactor only. No functional change. and removed cla: yes size/XL labels Feb 12, 2020
// deploymentInitializer detects a deployment type and is able to extract image names from it
type DeploymentInitializer interface {
// Initializer detects a deployment type and is able to extract image names from it
type Initializer interface {
Copy link
Contributor

@balopat balopat Feb 12, 2020

Choose a reason for hiding this comment

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

Why? I think this is focused on Deployments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it reads a little clearer now, since we're using build.Initializer and deploy.Initializer in the same code block

Copy link
Contributor

@balopat balopat left a comment

Choose a reason for hiding this comment

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

LGTM with nit: why rename the DeployInitializer?

@codecov
Copy link

codecov bot commented Feb 12, 2020

Codecov Report

Merging #3682 into master will decrease coverage by 0.01%.
The diff coverage is 82.92%.

Impacted Files Coverage Δ
pkg/skaffold/initializer/build/json.go 90% <100%> (+1.11%) ⬆️
pkg/skaffold/initializer/build/util.go 100% <100%> (ø)
pkg/skaffold/initializer/deploy/init.go 90% <100%> (ø) ⬆️
pkg/skaffold/initializer/config.go 100% <100%> (ø) ⬆️
pkg/skaffold/initializer/build/builders.go 57.69% <57.69%> (-35.11%) ⬇️
pkg/skaffold/initializer/init.go 50% <66.66%> (-8.7%) ⬇️
pkg/skaffold/initializer/build/cli.go 75% <75%> (ø)
pkg/skaffold/initializer/build/init.go 80.95% <80.95%> (ø)
pkg/skaffold/initializer/build/resolve.go 94.44% <94.44%> (ø)
... and 4 more

@nkubala nkubala merged commit 45a55e1 into GoogleContainerTools:master Feb 13, 2020
@nkubala nkubala deleted the init-refactor-controller branch February 13, 2020 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes refactor A PR that contains a refactor only. No functional change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants