-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
UPSTREAM: <drop>: fixup for 14537 #6409
UPSTREAM: <drop>: fixup for 14537 #6409
Conversation
daeee97
to
bbe26b2
Compare
[test] |
Unrelated flake?
|
re[test] |
Different flake:
|
[test] |
bbe26b2
to
078ce03
Compare
@markturansky FYI that wasn't the flake. The flake was #6259 (comment), which we're trying to fix with #6407 |
Thanks for the clarification. |
Latest failure:
[test] |
❄️ ⛄
[test] |
Evaluated for origin test up to 078ce03 |
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/7995/) |
woot! all green! |
@ncdc I think you asked me about it earlier. It isn't tagged for merge though. |
Going to merge so it makes a build afterwards but I want to see a LGTM and sign off from the upstream reviewer. [merge] |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/7995/) (Image: devenv-rhel7_2994) |
Evaluated for origin merge up to 078ce03 |
Merged by openshift-bot
if err != nil { | ||
newClaim, ok := newObj.(*api.PersistentVolumeClaim) | ||
if !ok { | ||
glog.Errorf("Expected PersistentVolumeClaim but handler received %+v", newObj) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this be a return? you go on to use newClaim
anyway, which seems wrong
Conflicts and rejected patch files means I missed a few things in the origin patch PR.
This PR cleans up a few issues and fixes one bug.
@deads2k @pweil-