Labelling this PR as size/L

GCE e2e test build/test passed for commit 7b3d9524ebdc5c4ac08f1aa31c918ec952cde890.

• Build Log
• Test Artifacts
• Internal Jenkins Results

GCE e2e test build/test passed for commit 405dd6d77fcfecdae8600b9e4234b202a1935ab6.

• Build Log
• Test Artifacts
• Internal Jenkins Results

@k8s-bot unit test this please

GCE e2e test build/test passed for commit 27fb62f0e7cf49725f2b8e9130b736962b01c1f6.

• Build Log
• Test Artifacts
• Internal Jenkins Results

GCE e2e test build/test passed for commit d2dfc91.

• Build Log
• Test Artifacts
• Internal Jenkins Results

cc @caesarxuchao - will affect your client generation :)

@caesarxuchao - I'm happy to change your client generator once this is merge (in a separate PR).

@krousey - can you please take a look (this is mostly mechanical PR)

In the middle of looking now.

On Tue, Dec 1, 2015 at 10:44 AM, Wojciech Tyczynski <
notifications@github.com> wrote:

@krousey https://github.com/krousey - can you please take a look (this
is mostly mechanical PR)

—
Reply to this email directly or view it on GitHub
#17863 (comment)
.

This seems like a drastic change in the public client interface that would break many users outside of our code base. Would you consider instead keeping the same Watch signature in the unversioned client and populating the options struct with the selector parameters before passing it to RESTClient's Watch?

I agree that this is a cleaner signature and preferred, but is this something we should get right in the versioned clients we're going to generate and just paper it over here?

cc @smarterclayton @liggitt

@wojtek-t Thanks! Or we can update the templates of client generator before we release the tool, it's impractical to update the generator every time there is a change to the client code.

This seems like a drastic change in the public client interface that would break many users outside of our code base. Would you consider instead keeping the same Watch signature in the unversioned client and populating the options struct with the selector parameters before passing it to RESTClient's Watch?
I agree that this is a cleaner signature and preferred, but is this something we should get right in the versioned clients we're going to generate and just paper it over here?

I don't think it's a drastic change. We already wanted to cleanup our codebase as part of 1.2 and this in my opinion is a step in a good direction.
Also, I don't think that it will break that many clients and fixing it is relatively simple (also note that we are not changing the official k8s API here).

@lavalamp @smarterclayton - I think we need your opinion on that.

I think we can live with this - it's a bit more of mechanical change, and i originally pushed for this so I don't mind taking a small hit :)

Continuous integration appears to have missed, closing and re-opening to trigger it

I think it's good to think carefully about whether we're breaking clients for a good reason or not, but we do have the client in an "unversioned" directory for a reason. In this case I'm happy if @smarterclayton is happy.

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

GCE e2e build/test failed for commit d2dfc91.

• Build Log
• Test Artifacts
• Internal Jenkins Results

@k8s-bot e2e test this please

GCE e2e test build/test passed for commit d2dfc91.

• Build Log
• Test Artifacts
• Internal Jenkins Results

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

GCE e2e test build/test passed for commit d2dfc91.

• Build Log
• Test Artifacts
• Internal Jenkins Results

Automatic merge from submit-queue