Skip to content

skipPush -> push#1114

Merged
dgageot merged 6 commits intoGoogleContainerTools:masterfrom
balopat:rename_push
Oct 9, 2018
Merged

skipPush -> push#1114
dgageot merged 6 commits intoGoogleContainerTools:masterfrom
balopat:rename_push

Conversation

@balopat
Copy link
Copy Markdown
Contributor

@balopat balopat commented Oct 8, 2018

Fixes #1113.

@balopat balopat mentioned this pull request Oct 8, 2018
11 tasks
@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 8, 2018

Codecov Report

Merging #1114 into master will increase coverage by 0.18%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1114      +/-   ##
==========================================
+ Coverage   42.96%   43.14%   +0.18%     
==========================================
  Files          74       77       +3     
  Lines        3403     3537     +134     
==========================================
+ Hits         1462     1526      +64     
- Misses       1801     1858      +57     
- Partials      140      153      +13
Impacted Files Coverage Δ
pkg/skaffold/build/local/types.go 0% <0%> (ø) ⬆️
pkg/skaffold/schema/v1alpha3/upgrade.go 68.75% <88.88%> (ø)
pkg/skaffold/schema/v1alpha3/defaults.go 40.65% <0%> (ø)
pkg/skaffold/schema/v1alpha3/config.go 45.45% <0%> (ø)

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 b809bd5...36c7332. Read the comment docs.

Copy link
Copy Markdown
Contributor

@dgageot dgageot left a comment

Choose a reason for hiding this comment

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

@balopat I like that. You should add the code that migrates existing configs though.

@balopat
Copy link
Copy Markdown
Contributor Author

balopat commented Oct 8, 2018

Thanks for catching that @dgageot! Done.

skaffold.yaml Outdated
- image: gcr.io/k8s-skaffold/skaffold
- imageName: gcr.io/k8s-skaffold/skaffold
docker:
dockerfile: deploy/skaffold/Dockerfile
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think dockerfile should be dockerfilePath

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

agreed - I changed it

@dgageot dgageot merged commit ee5e8f1 into GoogleContainerTools:master Oct 9, 2018
@balopat balopat deleted the rename_push branch October 15, 2018 22:19
# local:
# skipPush: true
# false by default for local clusters, true for remote clusters
# push: false
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The doc in the previous paragraph still talks about skip push, with inverted boolean value: this is confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants