Skip to content

Conversation

@bparees
Copy link

@bparees bparees commented Mar 18, 2020

also remove obsolete info

@bparees
Copy link
Author

bparees commented Mar 18, 2020

@gabemontero here's the refactor PR i promised.

Copy link
Owner

@gabemontero gabemontero left a comment

Choose a reason for hiding this comment

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

Just a few grammar/wording suggestions via suggested commits, as well as a call to remove CSI driver factoids and just reference the new PR (akin to removing broader scenario elements from the CSI driver PR and just reference this proposal).

Thanks for this rework and accelerating this along @bparees

reference from a Pod and may only be referenced in a Pod via a `PersistentVolumeClaim`
- To that end, one of the sidecar containers, the [external provisioner](https://kubernetes-csi.github.io/docs/external-provisioner.html),
helps with this
- To that end, one of the sidecar containers, the [external provisioner](https://kubernetes-csi.github.io/docs/external-provisioner.html), helps with this
Copy link
Owner

Choose a reason for hiding this comment

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

Similar to your suggestion in openshift/cluster-samples-operator#249, where we add a ref to this proposal and delete any repeat information, we should probably remove all these CSI factoids and simply add a ref to openshift/cluster-samples-operator#249, no?

Copy link
Author

Choose a reason for hiding this comment

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

yeah probably, can i leave that as a cleanup for you after you merge this? you have a better understanding of CSI than me at this point and are probably better positioned to decide what minimal info needs to be provided here, vs what to defer to the lower level CSI EP.

Copy link
Owner

Choose a reason for hiding this comment

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

yep that works for me

@bparees
Copy link
Author

bparees commented Mar 18, 2020

@gabemontero updates pushed other than dropping the CSI lifecycle bits

@gabemontero
Copy link
Owner

@gabemontero updates pushed other than dropping the CSI lifecycle bits

yep was just about to ask about that ;-) ... thanks @bparees

@gabemontero gabemontero merged commit 64d86b4 into gabemontero:bld-entitlements Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants