search

pmorie / osb-broker-lib

A go library for developing an Open Service Broker
Apache License 2.0
28 stars 23 forks source link

Expose implementations of platform specific identities #4

Closed shawn-hurley closed 6 years ago

shawn-hurley commented 6 years ago

Currently putting this into the broker package, I thought this was an okay place because callers already need to import the broker package to get the request context struct. Implementation is based on the spec definition. (edit: including the link to OSB API Definitions)

shawn-hurley commented 6 years ago

Depends on #9 to use the new constants in v2/constants.go from pmorie/go-open-service-broker-client#111

lilic commented 6 years ago

@shawn-hurley Seem like that PR was merged and got into the newest release, now you can bump up the dep on the client to 0.0.5 and then we can re-review this?

shawn-hurley commented 6 years ago

@LiliC If you look at #9 that has the code to bump the vendor. If that could be merged first then this should be good.

lilic commented 6 years ago

@shawn-hurley Sorry about that, misread the comment above!

shawn-hurley commented 6 years ago

@LiliC @pmorie just a ping to let you know the dependent PR has been merged

lilic commented 6 years ago

Also another thing is would be great to have an example use case in the osb-starter-pack broker for this. WDYT?

shawn-hurley commented 6 years ago

to have an example use case in the osb-starter-pack broker for this

I think that makes sense. The question I have is should that be added to the starter-pack after this is added to the lib and is released or should we create the example now with a dependency on this PR? I think developing that might be harder than the first way IMO

lilic commented 6 years ago

I think that makes sense. The question I have is should that be added to the starter-pack after this is added to the lib and is released or should we create the example now with a dependency on this PR? I think developing that might be harder than the first way IMO

IMO after this is released should be fine.

shawn-hurley commented 6 years ago

@LiliC @pmorie @n3wscott @kibbles-n-bytes @jmrodri

I have updated the implementation if you all could take a look and see if this is what you were expecting.

jmrodri commented 6 years ago

LGTM now

jmrodri commented 6 years ago

lgtm, what's left here?