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

fix go-jose imports #605

Closed
wants to merge 1 commit into from
Closed

fix go-jose imports #605

wants to merge 1 commit into from

Conversation

joshuarubin
Copy link

@joshuarubin joshuarubin commented Oct 9, 2017

This is a patch against v0.9.13, but there was no branch based on that at present, so I apologize for requesting this against master.

When using the hydra sdk in other go packages, there is a problem with the go-jose import that makes it impossible, unless the other package uses glide, to resolve the vendor issues such that hydra still builds and tests successfully while the package that imports it builds and tests successfully. This is because it uses the github.com/square/go-jose import while expecting the v1 version of go-jose which is supposed to be imported with gopkg.in/square/go-jose.v1.

This patch fixes the imports so that go-jose v2 (which uses the github.com/square/go-jose import`) can be used instead and drops the v1 requirement from glide.

Signed-off-by: Joshua Rubin <jrubin@zvelo.com>
@joshuarubin joshuarubin mentioned this pull request Oct 9, 2017
@aeneasr
Copy link
Member

aeneasr commented Oct 10, 2017

I created a new branch for 0.9.x: https://github.com/ory/hydra/tree/v0.9.x - can you rebase against that?

Also note that this might be an issue with fosite as well, go-jose is used there as well (version 1.0) so resolving this only here won't solve it properly (I think)

@joshuarubin joshuarubin changed the base branch from master to v0.9.x October 10, 2017 15:53
@joshuarubin
Copy link
Author

I haven't had an issue with fosite so far, but yes it can't hurt to add a Gopkg.toml there either.

I've rebased this as asked.

@aeneasr
Copy link
Member

aeneasr commented Oct 11, 2017

This can be closed right?

@joshuarubin
Copy link
Author

yes, it's included in #606

@joshuarubin joshuarubin deleted the fix_go_jose branch October 11, 2017 19:14
@aeneasr
Copy link
Member

aeneasr commented Oct 25, 2017

I made this change to master as well (and switched to dep) which will be released with #630

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