Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

Remove dependency on pkg/brokerapi #1240

Merged
merged 1 commit into from
Sep 20, 2017

Conversation

pmorie
Copy link
Contributor

@pmorie pmorie commented Sep 19, 2017

Remove the only dependency we have in production code on this package.

@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 19, 2017
@duglin
Copy link
Contributor

duglin commented Sep 19, 2017

Why would it not be better to let the brokerapi define this constant since if it changes in the future it would be nice to have it happen automagically for us since its really the brokerapi layer that knows what value to put in there no? Or at least the default value. I can see us allowing people to override it though.

@pmorie
Copy link
Contributor Author

pmorie commented Sep 19, 2017

@duglin the pkg/brokerapi package needs to go away from our production code - this is literally the only use of it. I'm fine pushing the constant into go-open-service-broker-api if that's more to your liking, but I want to eliminate the dependency on this package so we can move it out of pkg/ and into contrib/

WDYT

@MHBauer
Copy link
Contributor

MHBauer commented Sep 19, 2017

Current dependency is sort of a fake 'remote' dependency. Would prefer if we delegated to go-open-service-broker-api and the constant was defined there (along with other supported constants).

Copy link
Contributor

@MHBauer MHBauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lint failure

Found 1 lint suggestions; failing.
pkg/controller/controller.go:57:2: exported const ContextProfilePlatformKubernetes should have comment (or a comment on this block) or be unexported

Copy link
Contributor

@MHBauer MHBauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fine as is. Made upstream issue kubernetes-retired/go-open-service-broker-client#79 to track.

@MHBauer MHBauer merged commit b55c94e into kubernetes-retired:master Sep 20, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. LGTM1 size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants