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

Move Syncer into Deployer #6053

Merged
merged 1 commit into from
Jun 22, 2021
Merged

Conversation

nkubala
Copy link
Contributor

@nkubala nkubala commented Jun 21, 2021

Related: #5813

Description

This change moves the Syncer object from the Runner into the Deployer, since the syncing behavior is implicitly tied to the underlying Deployer implementation. See #5809 for a more detailed description of why we need to do this.

This change adds one method to the Deployer interface:

type Deployer interface {
  ...

  GetSyncer() sync.Syncer
}

The GetSyncer() method is used by the Runner to retrieve theSyncer implementation and actually orchestrate that behavior in the dev loop.

@nkubala nkubala requested review from yuwenma and a team as code owners June 21, 2021 22:57
@nkubala nkubala requested a review from MarlonGamez June 21, 2021 22:57
@google-cla google-cla bot added the cla: yes label Jun 21, 2021
@codecov
Copy link

codecov bot commented Jun 21, 2021

Codecov Report

Merging #6053 (592c3bd) into master (9ebb319) will decrease coverage by 0.04%.
The diff coverage is 48.38%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6053      +/-   ##
==========================================
- Coverage   70.31%   70.27%   -0.05%     
==========================================
  Files         471      473       +2     
  Lines       18077    18101      +24     
==========================================
+ Hits        12711    12720       +9     
- Misses       4434     4449      +15     
  Partials      932      932              
Impacted Files Coverage Δ
pkg/skaffold/deploy/deploy_mux.go 58.00% <0.00%> (-3.71%) ⬇️
pkg/skaffold/runner/v1/dev.go 72.30% <0.00%> (ø)
pkg/skaffold/runner/v1/runner.go 0.00% <ø> (ø)
pkg/skaffold/sync/syncer_mux.go 0.00% <0.00%> (ø)
pkg/skaffold/sync/types.go 0.00% <0.00%> (-100.00%) ⬇️
pkg/skaffold/deploy/helm/deploy.go 78.22% <50.00%> (-0.23%) ⬇️
pkg/skaffold/deploy/kpt/kpt.go 80.92% <50.00%> (-0.21%) ⬇️
pkg/skaffold/deploy/kubectl/kubectl.go 68.31% <50.00%> (-0.19%) ⬇️
pkg/skaffold/deploy/kustomize/kustomize.go 74.34% <50.00%> (-0.33%) ⬇️
pkg/skaffold/sync/provider.go 71.42% <71.42%> (ø)
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9ebb319...592c3bd. Read the comment docs.

@nkubala nkubala enabled auto-merge (squash) June 22, 2021 20:58
Copy link
Contributor

@MarlonGamez MarlonGamez left a comment

Choose a reason for hiding this comment

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

lgtm

@nkubala nkubala merged commit e88d988 into GoogleContainerTools:master Jun 22, 2021
@nkubala nkubala deleted the move-syncer branch June 22, 2021 21:20
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.

2 participants