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

chore(deps): bump github.com/knadh/koanf to v2.0.1 #693

Merged
merged 1 commit into from
Jun 9, 2023

Conversation

kralicky
Copy link
Contributor

@kralicky kralicky commented Jun 7, 2023

Related Issue or Design Document

This updates github.com/knadh/koanf from v1.4.4 to v2.0.1. There are no functional changes, but the module structure was reworked to have several submodules where there were previously packages within one main module.

The dependency of koanf v1 in this package is causing an issue for a module of mine that imports both koanf/v2 and a different ory package (which imports this one) because there is a new submodule in koanf/v2 with the same name as a package in v1. Go does not know how to resolve this, and it throws the following error:

error while importing github.com/knadh/koanf/v2: ambiguous import: found package github.com/knadh/koanf/maps in multiple modules:
	github.com/knadh/koanf v0.14.1-0.20201201075439-e0853799f9ec (/home/joe/go/pkg/mod/github.com/knadh/[email protected]/maps)
	github.com/knadh/koanf/maps v0.1.1 (/home/joe/go/pkg/mod/github.com/knadh/koanf/[email protected])

Replacing ory/x with my fork solves the problem.

Checklist

  • I have read the contributing guidelines and signed the CLA.
  • I have referenced an issue containing the design document if my change introduces a new feature.
  • I have read the security policy.
  • 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 approval (please contact [email protected]) from the maintainers to push the changes.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added the necessary documentation within the code base (if appropriate).

Further comments

@CLAassistant
Copy link

CLAassistant commented Jun 7, 2023

CLA assistant check
All committers have signed the CLA.

@kralicky kralicky changed the title Update koanf to use new submodule structure in v2 chore(deps): bump github.com/knadh/koanf to v2.0.1 Jun 7, 2023
@aeneasr
Copy link
Member

aeneasr commented Jun 7, 2023

This requires adopting the koanf changes in these other projects as well:

If it works there, we can merge this :)

@kralicky
Copy link
Contributor Author

kralicky commented Jun 7, 2023

@aeneasr Great - I'll do the same update for those repos as well, and link them here.

@kralicky
Copy link
Contributor Author

kralicky commented Jun 7, 2023

Each repo's go.mod contains a replacement to my fork of ory/x, which should allow it to build and run tests to see if they pass. I suppose we would have to merge this PR first and then edit the other ones to remove the replacement? Not sure, up to you.

@aeneasr aeneasr merged commit 27ed2da into ory:master Jun 9, 2023
@aeneasr
Copy link
Member

aeneasr commented Jun 9, 2023

Awesome, thank you! 🎉 Your contribution makes Ory better :)

@aeneasr
Copy link
Member

aeneasr commented Jun 9, 2023

Could you update the PRs in the individual repos now with the official ory/x version? :) Thank you! 👍

@kralicky
Copy link
Contributor Author

kralicky commented Jun 9, 2023

Updated the other PRs to import ory/x@master. 👍

@kralicky
Copy link
Contributor Author

kralicky commented Jun 9, 2023

I also went ahead and updated fosite as well (that's the library I'm using directly). Let me know if those changes look good. I made my best guess on some updates to error-related test cases.

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