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

Switch to go modules #65

Closed
wants to merge 2 commits into from
Closed

Conversation

RomanMinkin
Copy link
Contributor

@RomanMinkin RomanMinkin commented Dec 5, 2018

Signed-off-by: Roman Minkin roman.minkin@keysight.com

Fixes #64

UPDATE

There is a a temporary solution based on fork of this PR and following go.mod modification:

module my-package

require (
    github.com/ory/keto v1.0.0-beta.9
)

replace github.com/ory/keto => github.com/RomanMinkin/keto v1.0.0-beta.9.2

Proposed changes

Checklist

  • I have read the contributing guidelines
  • I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security
    vulnerability, I confirm that I got green light (please contact hi@ory.sh) from the maintainers to push the changes.
  • I signed the Developer's Certificate of Origin
    by signing my commit(s). You can amend your signature to the most recent commit by using git commit --amend -s. If you
    amend the commit, you might need to force push using git push --force HEAD:<branch>. Please be very careful when using
    force push.
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation within the code base (if appropriate)
  • I have documented my changes in the developer guide (if appropriate)

Further comments

I know that there is an undergoing effort to switch to go mod under #48 but at the current time it's impossible to use Keto as a dependency.

if it can not be merged into master is it possible to merge it into a secondary branch so I still can resolve Keto correctly.

Thank you!

Signed-off-by: Roman Minkin <roman.minkin@keysight.com>
@RomanMinkin
Copy link
Contributor Author

I see that the problem is actually bigger, since github.com/ory/go-convenience/corsx imported in cmd and for whatever reason is not picked up by go mod

@aeneasr
Copy link
Member

aeneasr commented Dec 6, 2018

Hi there, thank you for the fix! I'm actually in the process of wrapping up #48 next week which will also address this issue. Would it be ok for you to use the docker/binaries until then?

@sum2000
Copy link
Contributor

sum2000 commented Dec 6, 2018

@aeneasr how exactly is #48 related to this PR?

@aeneasr
Copy link
Member

aeneasr commented Dec 6, 2018

Nevermind, probably better to get this one through quickly and then merge #48 . But it would be better to stay with dep for now. #48 makes the switch to go modules.

@aeneasr
Copy link
Member

aeneasr commented Dec 6, 2018

Please see: #64 (comment)

If you're able to run this fine, this pr can be closed.

@sum2000
Copy link
Contributor

sum2000 commented Dec 6, 2018

@aeneasr could you please elaborate on why is it better to stay with dep? Hydra switched to go mod and since keto is still on dep, it seems impossible to make the latest versions of both work together.

@aeneasr
Copy link
Member

aeneasr commented Dec 6, 2018

Because we're switching to go modules with #48 and I don't see a lot of benefits in putting effort into this when we'll merge this next week anyways!

@sum2000
Copy link
Contributor

sum2000 commented Dec 6, 2018

@aeneasr I see your point! Thanks. Do you plan on making keto compatible with hydra release 1.0.0-rc.2? Currently, they rely on a different version of the same packages (For ex., ory fosite)

Signed-off-by: Roman Minkin <roman.minkin@keysight.com>
@RomanMinkin
Copy link
Contributor Author

@aeneasr We found a temporary solution #64 (comment)

p.s also added explanation to the PR description

cc @sum2000

@aeneasr
Copy link
Member

aeneasr commented Dec 7, 2018

Closed in favor of #48

@aeneasr aeneasr closed this Dec 7, 2018
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.

Keto can not be consumed as a dependency
3 participants