-
Notifications
You must be signed in to change notification settings - Fork 4.8k
kube rebase 4c8e6f47ec23f390978e651232b375f5f9cde3c7 #5143
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
Conversation
|
@pweil- review |
Types/conversions/copies lgtm which is where I suspect the conflicts arose since most of the rest was a straight add. |
|
@soltysh checkout out the conflicts and changes to the carries required to handle the change to |
|
@smarterclayton please review |
|
@pweil- I want to collapse all scc pulls into a single carry. Is there a reason not to do that? |
I thought you and @smarterclayton were in a commit race... |
I would gladly trade all my commits to never look at a rebase again! |
nope, other than the provider pieces not compiling right away due to dependent api changes that are carried |
|
Those changes are ok, you probably want to cherry pick smarterclayton@99ee790 instead |
|
COMPILING! Got the rebase compiling, though I'm short one patch, so I have commit to revert and fix up the last two packages. @soltysh Can you start getting the kube unit tests passing? I haven't picked all the patches yet, but what we have should pass the tests. |
|
@deads2k will start with that in the morning |
Thanks. Fixing imports and all for the origin unit tests to compile (they won't run successfully yet) would be helpful too. I'll do the picks myself to keep up with my lists and track how badly all the changes conflict for requesting reviews. |
|
@deads2k here are my efforts deads2k#18 |
|
@ironcladlou checkout the "DANMACE REVIEW" commit and let me know. |
|
It's not an obvious fix, so carry it. On Oct 16, 2015, at 10:10 AM, David Eads notifications@github.com wrote: Wall of Shame
Followups
Upstreams 8009272
bump(k8s.io/kubernetes) 86b4e77You can view, comment on, or merge this pull request online at: #5143
File Changes
Patch Links:
— |
|
deads2k@00ffd6b LGTM |
|
I hit a serious snag: kubernetes/kubernetes#15781 . I'm going to see if I can work around it without reverting, but we make extensive use of shortnames. |
|
|
|
|
|
|
f574d4f to
b0e703e
Compare
We already set it: https://github.com/openshift/origin/blob/master/cmd/genconversion/conversion.go#L46. I'll add a followup to figure out why we needed to do it. |
|
we're getting generated conversions when we already registered our own in subpackages. For example: Hopefully fuzzing and hand-written tests are ensuring we're using our non-generated conversions edit: this is scary... it means an autogenerated conversion chain can prevent a custom conversion function from being called... there are now lots of latent autoconvert_* functions, which, if called, would be incorrect. Yay fuzz tests? |
With the introduction of |
|
re[test] |
|
Evaluated for origin test up to 1eb472b |
|
LGTM, add follow up to remove apply annotation |
Knowledge only makes you sad |
|
green. |
|
[vipmerge]! |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/5939/) (Image: devenv-fedora_2496) |
|
Evaluated for origin vipmerge up to 1eb472b |
Merged by openshift-bot
|
After this I'm unable to generate v1beta3 conversions |
|
Generate creates an incorrect mapping. |
|
Nevermind, this happens if you have |
|
@deads2k did you remove the fake factory? I was using it in a couple of tests that still haven't merged... |
Yeah, it was dead and had several conflicts that weren't worth fixing for code dead code. Sorry. |
Wall of Shame
Followups
UPSTREAM: <carry>: Disable --validate by defaultis incorrect. It should only change our wrapped methods, not the originals. [post-rebase] disabling --validate is too broad #5235test/cmd/admin.sh"not present in output" tests check only the expected field [post-rebase] make test/cmd/admin.sh "not present in output" tests check only the expected field #5236UPSTREAM: <drop>: make test pass with old codecis bad. We need to find the client codec priority issue in the test. [post-rebase] remove UPSTREAM: <drop>: make test pass with old codec #5237UPSTREAM: <drop>: tweak generator to handle conversions in other packagesshouldn't be necessary withAssumePrivateConversions. Figure out why it's needed. [post-rebase] conversion generator is broken #5238Upstreams
8009272 bump(k8s.io/kubernetes): 4c8e6f4
2009660 bump(k8s.io/kubernetes) 86b4e77