Skip to content
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

Go 1.7 + Vendor jwt-go at v2.7 #9

Merged
merged 2 commits into from
Aug 4, 2016
Merged

Go 1.7 + Vendor jwt-go at v2.7 #9

merged 2 commits into from
Aug 4, 2016

Conversation

VojtechVitek
Copy link
Contributor

@VojtechVitek VojtechVitek commented Jul 11, 2016

Breaks the API. We should release the Go 1.6 compatible version before merge.

@VojtechVitek VojtechVitek changed the title Go 1.7 Go 1.7 + Vendor jwt-go at v2.7 Jul 11, 2016
@pkieltyka pkieltyka merged commit 30d5d5b into master Aug 4, 2016
@pkieltyka pkieltyka deleted the go1.7 branch August 4, 2016 14:23
@advdv
Copy link

advdv commented Jan 10, 2017

I Would like to put this merge request up for discussion. Vendoring dependencies inside go libraries is discourages. Basically when I now vendor github.com/goware/jwtauth and next to that I vendor github.com/dgrijalva/jwt-go at version 2.7 myself the 'inner version' of jwtauth takes precedence and makes jwt tokens inside the request context unusable:

interface conversion: interface {} is *jwt.Token(github.com/dgrijalva/jwt-go/vendor/github.com/goware/jwtauth), not *jwt.Token(github.com/goware/jwtauth)

A more in-depth discussion can be found here: Masterminds/glide#303

In my opinion a warning at the top of the README is enough and this library should not include its own vendor directory.

@pkieltyka
Copy link
Member

@advanderveer I agree with you, I don't think the Go dep tools are suited for vendor folders from deps of deps

@VojtechVitek
Copy link
Contributor Author

@advanderveer We use govendor as dep tool and it resolves the inner dependencies just fine. This project is only compatible with 2.7, that's why we added the dependency explicitly.

Since glide can't recognize two exact same versions (well, it should) -- can you just remove vendor/ dir from this dependency by hand?

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.

3 participants