Skip to content

Go Bindings Canonical Import Path #156

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

Conversation

akutz
Copy link
Contributor

@akutz akutz commented Nov 23, 2017

This patch fixes #142 and adds a Go canonical import path of "github.com/container-storage-interface/spec/lib/go/csi" to the CSI Go bindings.

@jdef @saad-ali

This patch fixes container-storage-interface#142 and adds a Go canonical import path of
"github.com/container-storage-interface/spec/lib/go/csi" to the CSI Go
bindings.
@akutz
Copy link
Contributor Author

akutz commented Nov 23, 2017

I'm closing this and we should abandon the idea of canonical imports in #142 as they are not compatible with vendored dependencies. Instead Golang dep should be used to enforce the official CSI bindings.

@akutz akutz closed this Nov 23, 2017
@akutz akutz deleted the feature/go-bindings-canonical-import branch November 23, 2017 22:13
@akutz akutz restored the feature/go-bindings-canonical-import branch November 27, 2017 18:52
@akutz akutz reopened this Nov 27, 2017
@cpuguy83
Copy link
Contributor

There's a couple of ways we can deal with this.
If we specify go_package in the protobufs this should give us a canonical import.
Alternatively I believe we only need the canonical import path in one file in the package, in which case we can just place that in something like lib/go/csi/imports.go or something with just a package delcaration with the canonical import path.

However I would prefer if we don't modify the generated proto files after the fact.

@jdef jdef added the lifecycle/stale PR has been inactive for 90+ days. label Oct 4, 2018
@saad-ali
Copy link
Member

saad-ali commented Nov 7, 2018

We don't have resources to implement a redirect service at this time, and that would be required with this change. Considering 1.0 is coming up, this is not a priority.

@saad-ali saad-ali closed this Nov 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/stale PR has been inactive for 90+ days.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lang: Official CSI bindings should live in this repo
4 participants