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

Implement did generator #3

Merged
merged 19 commits into from
Jun 10, 2024
Merged

Implement did generator #3

merged 19 commits into from
Jun 10, 2024

Conversation

anjastrunk
Copy link
Contributor

No description provided.

Signed-off-by: Anja Strunk <anja.strunk@cloudandheat.com>
@anjastrunk anjastrunk added the SCS-VP10 Related to tender lot SCS-VP10 label May 30, 2024
@anjastrunk anjastrunk self-assigned this May 30, 2024
This was referenced May 30, 2024
Copy link
Member

@martinmo martinmo left a comment

Choose a reason for hiding this comment

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

Reviewed and tested on the CLI locally (which works 👍). In addition to my inline comments, I have two general suggestions:

  • Re-add the README that was part of the original PR that was closed in favor of this one.
  • Make it easier to "demo" for someone:
    • In order to try the CLI, I had to create a certificate and a key. For this, I reused the test data in created in the DidGenTestCase and exported them to files via interactive Python console, which was too cumbersome IMHO.
    • What can I do with the resulting DID json file? Can I feed it to some other tool?

The last question also makes me think if the DID creation test pipeline could benefit from integration testing with another tool – not maintained by us – which reads a DID and does something with it.

@martinmo
Copy link
Member

martinmo commented Jun 3, 2024

  • Re-add the README that was part of the original PR that was closed in favor of this one.

Adding to that: I suggest adding a link to the following two documents, which I found helpful for getting to know Decentralized Identifiers and the context in which they are used: https://w3c-ccg.github.io/did-primer/ and https://w3c.github.io/did-use-cases/ (for the latter one, the "Use Cases" section was helpful in my opinion).

anjastrunk and others added 6 commits June 4, 2024 14:28
Co-authored-by: Martin Morgenstern <martin.morgenstern@cloudandheat.com>
Signed-off-by: anjastrunk <119566837+anjastrunk@users.noreply.github.com>
Co-authored-by: Martin Morgenstern <martin.morgenstern@cloudandheat.com>
Signed-off-by: anjastrunk <119566837+anjastrunk@users.noreply.github.com>
Co-authored-by: Martin Morgenstern <martin.morgenstern@cloudandheat.com>
Signed-off-by: anjastrunk <119566837+anjastrunk@users.noreply.github.com>
Co-authored-by: Martin Morgenstern <martin.morgenstern@cloudandheat.com>
Signed-off-by: anjastrunk <119566837+anjastrunk@users.noreply.github.com>
Co-authored-by: Martin Morgenstern <martin.morgenstern@cloudandheat.com>
Signed-off-by: anjastrunk <119566837+anjastrunk@users.noreply.github.com>
Co-authored-by: Martin Morgenstern <martin.morgenstern@cloudandheat.com>
Signed-off-by: anjastrunk <119566837+anjastrunk@users.noreply.github.com>
@anjastrunk
Copy link
Contributor Author

* Re-add the README that was part of the original PR that was closed in favor of this one.

You are completely right, I forgot to copy README

* Make it easier to "demo" for someone:
  
  * In order to try the CLI, I had to create a certificate and a key. For this, I reused the test data in created in the `DidGenTestCase` and exported them to files via interactive Python console, which was too cumbersome IMHO.
  * What can I do with the resulting DID json file? Can I feed it to some other tool?

Details of DID, DID document and what they can be used for, are explained in W3C DID standard. Of course, without the new README, this is not obvious.

The last question also makes me think if the DID creation test pipeline could benefit from integration testing with another tool – not maintained by us – which reads a DID and does something with it.

Good point. DID document is required for Gaia-X and its infrastructure is not yet up and running. I will put this in backlog.

Signed-off-by: Anja Strunk <anja.strunk@cloudandheat.com>
Signed-off-by: Anja Strunk <anja.strunk@cloudandheat.com>
Signed-off-by: Anja Strunk <anja.strunk@cloudandheat.com>
Signed-off-by: Anja Strunk <anja.strunk@cloudandheat.com>
@anjastrunk anjastrunk requested a review from martinmo June 4, 2024 13:17
@anjastrunk
Copy link
Contributor Author

@martinmo I added your changes and suggestions please re-review.

Signed-off-by: Anja Strunk <anja.strunk@cloudandheat.com>
Signed-off-by: Anja Strunk <anja.strunk@cloudandheat.com>
Signed-off-by: Anja Strunk <anja.strunk@cloudandheat.com>
Copy link

@mbuechse mbuechse left a comment

Choose a reason for hiding this comment

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

very minor points

anjastrunk and others added 2 commits June 6, 2024 08:22
Co-authored-by: Matthias Büchse <github@mbue.de>
Signed-off-by: anjastrunk <119566837+anjastrunk@users.noreply.github.com>
Co-authored-by: Matthias Büchse <github@mbue.de>
Signed-off-by: anjastrunk <119566837+anjastrunk@users.noreply.github.com>
anjastrunk and others added 3 commits June 6, 2024 08:22
Co-authored-by: Matthias Büchse <github@mbue.de>
Signed-off-by: anjastrunk <119566837+anjastrunk@users.noreply.github.com>
Signed-off-by: Anja Strunk <anja.strunk@cloudandheat.com>
Signed-off-by: anjastrunk <119566837+anjastrunk@users.noreply.github.com>
Copy link

github-actions bot commented Jun 6, 2024

Code Coverage

Package Line Rate Health
creator 92%
tests 99%
Summary 96% (210 / 218)

Minimum allowed line rate is 90%

Copy link
Member

@martinmo martinmo left a comment

Choose a reason for hiding this comment

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

Second round of review done – I think this is good to go!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SCS-VP10 Related to tender lot SCS-VP10
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants