remove deploymentconfig registry #15858
Conversation
openshift-merge-robot
added
the
size/L
openshift-merge-robot
added
approved
deads2k
force-pushed
the
server-33-storage
branch
from
4cc2551 to
5ca1ade
Compare
openshift-merge-robot
removed
the
needs-rebase
|
/retest |
2 similar comments
|
/retest |
|
/retest |
deads2k
force-pushed
the
server-33-storage
branch
from
5ca1ade to
0d12974
Compare
|
/retest |
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return deployapi.ScaleFromConfig(deploymentConfig), nil | ||
| return deployapi.ScaleFromConfig(deploymentConfig.(*deployapi.DeploymentConfig)), nil |
There was a problem hiding this comment.
i guess we don't need to check this assertion?
| if err != nil { | ||
| return nil, false, errors.NewNotFound(extensions.Resource("scale"), name) | ||
| } | ||
| deploymentConfig := uncastObj.(*deployapi.DeploymentConfig) |
There was a problem hiding this comment.
check?
It would be programmer error. I don't feel too bad about panic-ing here.
| @@ -30,7 +30,7 @@ import ( | |||
| "github.com/openshift/origin/pkg/build/webhook/github" | |||
| "github.com/openshift/origin/pkg/build/webhook/gitlab" | |||
| deployapiv1 "github.com/openshift/origin/pkg/deploy/apis/apps/v1" | |||
| deployconfigregistry "github.com/openshift/origin/pkg/deploy/registry/deployconfig" | |||
| oappsclient "github.com/openshift/origin/pkg/deploy/generated/internalclientset" | |||
There was a problem hiding this comment.
nit: why not appsclient?
collides with upstream
| return nil, err | ||
| } | ||
| deployRollbackClient := deployrollback.Client{ | ||
| GRFn: deployrollback.NewRollbackGenerator().GenerateRollback, |
| @@ -1,135 +0,0 @@ | |||
| package etcd | |||
There was a problem hiding this comment.
@deads2k for clarification, we don't need this test anymore or it moves elsewhere or we already test this in different place?
There was a problem hiding this comment.
@deads2k for clarification, we don't need this test anymore or it moves elsewhere or we already test this in different place?
We don't need it because its just re-running the generic etcd storage testing.
|
Looks like you snuck some route and template registry removal in here too, you sly dog |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, ironcladlou The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
/retest |
|
The failed test does not seem to be related to my change |
|
Automatic merge from submit-queue (batch tested with PRs 15942, 15940, 15957, 15858, 15946) |
Remove more registry dependencies from "easy" resources.