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.

Sign up for GitHub

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

Jump to bottom

update client.Config to use GroupVersion #17316

Merged
merged 1 commit into from
Nov 24, 2015
Merged

update client.Config to use GroupVersion #17316

merged 1 commit into from
Nov 24, 2015
+314 −224

Conversation

deads2k
Copy link
Contributor

@deads2k deads2k commented Nov 16, 2015

Updates the ClientConfig to use the actual type.

@liggitt @caesarxuchao

@deads2k deads2k mentioned this pull request Nov 16, 2015
Closed
61 tasks
@k8s-github-robot
Copy link

Labelling this PR as size/XL

@k8s-github-robot k8s-github-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Nov 16, 2015
@k8s-bot
Copy link

k8s-bot commented Nov 16, 2015

GCE e2e test build/test passed for commit 4134e740f137e39a09862abfed4ee4a86e3ff8bb.

@deads2k deads2k force-pushed the gv-ClientForVersion branch from 4134e74 to 09092d2 Compare November 16, 2015 16:54
@k8s-bot
Copy link

k8s-bot commented Nov 16, 2015

GCE e2e test build/test passed for commit 09092d2d3445318c0d329d4e73a05ad698c2623f.

@deads2k deads2k closed this Nov 16, 2015
@deads2k deads2k reopened this Nov 16, 2015
@deads2k
Copy link
Contributor Author

deads2k commented Nov 16, 2015

@k8s-bot unit test this please

@caesarxuchao
Copy link
Member

ref #17216

@caesarxuchao caesarxuchao reopened this Nov 16, 2015
@caesarxuchao
Copy link
Member

ref #17216

Sorry for the slip..

@caesarxuchao caesarxuchao mentioned this pull request Nov 16, 2015
Merged
@deads2k
Copy link
Contributor Author

deads2k commented Nov 16, 2015

@k8s-bot unit test this please

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 16, 2015
liggitt
liggitt reviewed Nov 17, 2015
View reviewed changes
pkg/client/unversioned/client_test.go
@@ -85,8 +85,8 @@ func (c *testClient) Setup(t *testing.T) *testClient {
version = testapi.Default.Version()
Copy link
Member

Choose a reason for hiding this comment

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

c.Version is never set (the field is only used on line 83 and 94). Remove c.Version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

c.Version is never set (the field is only used on line 83 and 94). Remove c.Version?

version is used on 89

Copy link
Contributor Author

Choose a reason for hiding this comment

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

c.Version is never set (the field is only used on line 83 and 94). Remove c.Version?

wait, why do they have this lever?

@deads2k deads2k force-pushed the gv-ClientForVersion branch from 09092d2 to 543a389 Compare November 17, 2015 13:48
@deads2k
Copy link
Contributor Author

deads2k commented Nov 17, 2015

comments addressed.

@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 17, 2015
@k8s-bot
Copy link

k8s-bot commented Nov 17, 2015

GCE e2e build/test failed for commit 543a389adf9dd9e3db8c43569dd8961f673cf715.

@deads2k
Copy link
Contributor Author

deads2k commented Nov 17, 2015

@k8s-bot unit test this rudely

@wojtek-t
Copy link
Member

@deads2k - it seems there is some compile error in your PR:

"/go/src/k8s.io/kubernetes/test/integration/framework/master_utils.go:99: unknown "k8s.io/kubernetes/pkg/client/unversioned".Config field 'Version' in struct literal"

@deads2k deads2k force-pushed the gv-ClientForVersion branch from 543a389 to 0c211b8 Compare November 17, 2015 20:07
@k8s-github-robot
Copy link

PR changed after LGTM, removing LGTM.

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 17, 2015
@deads2k deads2k added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 17, 2015
@deads2k
Copy link
Contributor Author

deads2k commented Nov 17, 2015

updated for integration compilation consistent with unit and e2e tests. re-adding lgtm

@k8s-bot
Copy link

k8s-bot commented Nov 17, 2015

GCE e2e test build/test passed for commit 0c211b84f9287df9c8cd239af34886f15dc82c2f.

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 20, 2015
@deads2k deads2k force-pushed the gv-ClientForVersion branch from 0c211b8 to 3768922 Compare November 20, 2015 18:13
@deads2k
Copy link
Contributor Author

deads2k commented Nov 20, 2015

rebased, no significant changes.

@k8s-github-robot
Copy link

PR changed after LGTM, removing LGTM.

@k8s-github-robot k8s-github-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 20, 2015
@deads2k deads2k added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 20, 2015
@k8s-bot
Copy link

k8s-bot commented Nov 20, 2015

GCE e2e test build/test passed for commit 376892292aa4846913bfbbe42ae2770881a9a115.

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 21, 2015
@deads2k deads2k force-pushed the gv-ClientForVersion branch from 3768922 to a87d927 Compare November 21, 2015 13:29
@k8s-github-robot
Copy link

PR changed after LGTM, removing LGTM.

@k8s-github-robot k8s-github-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 21, 2015
@k8s-bot
Copy link

k8s-bot commented Nov 21, 2015

GCE e2e test build/test passed for commit a87d927.

@deads2k deads2k added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 21, 2015
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Nov 24, 2015

GCE e2e test build/test passed for commit a87d927.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

k8s-github-robot pushed a commit that referenced this pull request Nov 24, 2015
Merge pull request #17316 from deads2k/gv-ClientForVersion
4fd6b6d 
Auto commit by PR queue bot
@k8s-github-robot k8s-github-robot merged commit 4fd6b6d into kubernetes:master Nov 24, 2015
@deads2k deads2k deleted the gv-ClientForVersion branch February 26, 2016 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@liggitt liggitt

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@deads2k @k8s-github-robot @k8s-bot @caesarxuchao @liggitt @wojtek-t @k8s-teamcity-mesosphere @googlebot