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

Please expedite: the rarely attempted interface{} -> runtime.Object rename #1202

Merged
merged 8 commits into from
Sep 8, 2014

Conversation

lavalamp
Copy link
Member

@lavalamp lavalamp commented Sep 6, 2014

No description provided.

@lavalamp lavalamp force-pushed the fixApi3 branch 3 times, most recently from 3031597 to cdca8fa Compare September 8, 2014 05:13
@lavalamp lavalamp force-pushed the fixApi3 branch 2 times, most recently from 835e9b3 to 1d431ce Compare September 8, 2014 05:23
@lavalamp lavalamp changed the title WIP: large type rename incoming. Please expedite: the rarely attempted interface{} -> runtime.Object rename Sep 8, 2014
@lavalamp
Copy link
Member Author

lavalamp commented Sep 8, 2014

@smarterclayton @brendandburns @thockin @dchen1107: If you guys could avoid merging too much stuff before going through this one, that will make my life easier. I don't want to rebase this any more than absolutely necessary...

@smarterclayton
Copy link
Contributor

Other than that concern, rest of it looks fine and I'll be able to give a LGTM

@lavalamp
Copy link
Member Author

lavalamp commented Sep 8, 2014

@smarterclayton We could (in the future) make the method useful, like it could return an interface to the JSONBase and let us get rid of all the ResourceVersioner weirdness. That's a simple find-and-replace operation after this PR, so I'd rather get this in and then debate it.

@smarterclayton
Copy link
Contributor

Ok, then next question - can you make this a method on JSONBase instead so you don't need to define it on all the other objects (since JSONBase is embedded)?

@lavalamp
Copy link
Member Author

lavalamp commented Sep 8, 2014

@smarterclayton Wouldn't that be nice. Unfortunately, I tried it and that has the effect of forcing not a pointer. There's no way to force pointer on the object you're embedded in by adding a method on a thing that gets embedded. :( :(

@smarterclayton
Copy link
Contributor

The wailing and gnashing of teeth.... ok, LGTM. In the future would like to make the method more meaningful and its presence less odious.

lavalamp added a commit that referenced this pull request Sep 8, 2014
Please expedite: the rarely attempted interface{} -> runtime.Object rename
@lavalamp lavalamp merged commit 1789425 into kubernetes:master Sep 8, 2014
@lavalamp
Copy link
Member Author

lavalamp commented Sep 8, 2014

@smarterclayton Thanks for the quick feedback. Self-merging to avoid rebase hell.

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.

2 participants