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

Switch the API implementation to use the local APIs everywhere we can. #1544

Merged
merged 3 commits into from
Jun 14, 2017

Conversation

dlorenc
Copy link
Contributor

@dlorenc dlorenc commented May 31, 2017

No description provided.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 31, 2017
Copy link
Contributor

@r2d4 r2d4 left a comment

Choose a reason for hiding this comment

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

The change LGTM, but looks like you need to also change the associated tests.

@codecov-io
Copy link

codecov-io commented Jun 7, 2017

Codecov Report

Merging #1544 into master will decrease coverage by 0.15%.
The diff coverage is 26.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1544      +/-   ##
==========================================
- Coverage   39.52%   39.36%   -0.16%     
==========================================
  Files          50       50              
  Lines        2548     2548              
==========================================
- Hits         1007     1003       -4     
- Misses       1375     1378       +3     
- Partials      166      167       +1
Impacted Files Coverage Δ
pkg/minikube/machine/client_linux.go 28.57% <ø> (+7.51%) ⬆️
cmd/minikube/cmd/root.go 67.79% <ø> (-0.54%) ⬇️
cmd/minikube/cmd/mount.go 5% <0%> (ø) ⬆️
cmd/minikube/cmd/logs.go 21.42% <0%> (ø) ⬆️
cmd/minikube/cmd/service_list.go 11.53% <0%> (ø) ⬆️
cmd/minikube/cmd/ip.go 11.76% <0%> (ø) ⬆️
cmd/minikube/cmd/start.go 17.05% <0%> (ø) ⬆️
cmd/minikube/cmd/stop.go 13.33% <0%> (ø) ⬆️
cmd/minikube/cmd/ssh.go 10.52% <0%> (ø) ⬆️
cmd/minikube/cmd/env.go 64.39% <0%> (ø) ⬆️
... and 5 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 ae51e5f...96fdc2c. Read the comment docs.

@dlorenc
Copy link
Contributor Author

dlorenc commented Jun 8, 2017

@r2d4 ready for another look.

@dlorenc
Copy link
Contributor Author

dlorenc commented Jun 8, 2017

@minikube-bot test this pleas

@dlorenc
Copy link
Contributor Author

dlorenc commented Jun 8, 2017

@minikube-bot test this please

Copy link
Contributor

@r2d4 r2d4 left a comment

Choose a reason for hiding this comment

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

LGTM

@dlorenc
Copy link
Contributor Author

dlorenc commented Jun 9, 2017

@minikube-bot test this please

1 similar comment
@dlorenc
Copy link
Contributor Author

dlorenc commented Jun 9, 2017

@minikube-bot test this please

@dlorenc dlorenc merged commit 09f683b into kubernetes:master Jun 14, 2017
@dlorenc dlorenc deleted the local branch June 14, 2017 16:45
@aaron-prindle
Copy link
Contributor

Was this passing the none driver test? It seems that no entry in ~/.minikube/machines being made now when using the none driver, trying to identify if this is related.

@dlorenc
Copy link
Contributor Author

dlorenc commented Jun 15, 2017

Was this passing the none driver test? It seems that no entry in ~/.minikube/machines being made now when using the none driver, trying to identify if this is related.

I think it did, but I guess we lose access to the history now that it's merged :(

@r2d4
Copy link
Contributor

r2d4 commented Jun 15, 2017

You can click on the "details" button on the merge history message in the PR and see the test history. Looks like the None driver was passing

https://storage.googleapis.com/minikube-builds/logs/1544/Linux-None.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants