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

feat: separate DID Method trait from Create trait for create-able DIDs #157

Merged

Conversation

enmand
Copy link
Contributor

@enmand enmand commented May 5, 2024

Some DID methods allow for DID creation (e.g. with some key pair) and some have more infrastructure requirements around them (like did:web). This PR separates the trait for "create-able" DIDs, so that DID methods like did:web don't have to carry around trait types and no-implementation-implementations for trait functions they don't need.

@enmand
Copy link
Contributor Author

enmand commented May 5, 2024

On second though, maybe there is value in having all the methods defined in a single trait, even if it's a bit cumbersome for some method implementations -- I could see a world where there are separate e.g. Resolve, Create, Update and Deactivate trait but I also see value in having all the methods under a single trait

@KendallWeihe
Copy link
Contributor

I'm a little torn on this and need to think about it. I think it's a great idea to come to an implementation wherein the lack of support of a feature is determinable at build-time, as opposed to how we currently have things which wouldn't be discovered until runtime. However, strictly aligning to the specification I view all of the various CRUD operations on a method as a bundle of features and so at face value it makes sense for them to be in a single trait. I think... probably what you have proposed here @enmand is ideal, but let's let it sit for a bit and we can contemplate 🤔

Copy link
Contributor

@KendallWeihe KendallWeihe left a comment

Choose a reason for hiding this comment

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

let's do it!

@KendallWeihe KendallWeihe merged commit bc357fa into decentralized-identity:main May 9, 2024
9 checks passed
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