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

refactor(anoncreds)!: combine anoncreds packages #1723

Merged

Conversation

TimoGlastra
Copy link
Contributor

Combines the two anoncreds packages into @credo-ts/anoncreds to hopefully simplify setup a bit

Signed-off-by: Timo Glastra <timo@animo.id>
Copy link
Contributor

@genaris genaris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will certainly simplify agent setup. There are a few things that maybe can be improved or simplified later on, like the injection symbols usage (not sure if it's worth to keep using them if we'll support a single implementation) and the test structure, but I think it is perfectly fine for now.

README.md Outdated Show resolved Hide resolved
packages/anoncreds/src/AnonCredsModuleConfig.ts Outdated Show resolved Hide resolved
packages/anoncreds/src/AnonCredsModuleConfig.ts Outdated Show resolved Hide resolved
@@ -40,6 +42,11 @@ export class AnonCredsModule implements Module {
dependencyManager.registerSingleton(AnonCredsLinkSecretRepository)
dependencyManager.registerSingleton(AnonCredsRevocationRegistryDefinitionRepository)
dependencyManager.registerSingleton(AnonCredsRevocationRegistryDefinitionPrivateRepository)

// TODO: should we allow to override the service?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be useful, but we'll still being importing anoncreds-rs native library, so it would be still inconvenient if we really want to use other library.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but a model could exist where we make passing anoncreds optional and you provide your own services implementation. You'll only depend on the HL/anoncreds-shared package in this case which doesn't include the heavy AnonCreds-rs

@TimoGlastra TimoGlastra merged commit 9da02d4 into openwallet-foundation:main Feb 1, 2024
7 checks passed
@TimoGlastra TimoGlastra deleted the refactor/combine-anoncreds branch February 1, 2024 07:23
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