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

verifiable credential example #128

Merged
merged 3 commits into from
Jul 6, 2022
Merged

Conversation

andorsk
Copy link
Contributor

@andorsk andorsk commented Jul 4, 2022

please merge #126 first then this will follow:

➜  vc git:(vc-example) go run vc.go
Created Verifiable Credential:
 {
  "@context": [
    "https://www.w3.org/2018/credentials/v1",
    "https://www.w3.org/2018/credentials/examples/v1"
  ],
  "id": "http://example.edu/credentials/1872",
  "type": [
    "VerifiableCredential",
    "AlumniCredential"
  ],
  "issuer": "https://example.edu/issuers/565049",
  "issuanceDate": "2010-01-01T19:23:24Z",
  "credentialSubject": {
    "alumniOf": {
      "id": "did:example:c276e12ec21ebfeb1f712ebc6f1",
      "name": [
        {
          "lang": "en",
          "value": "Example University"
        },
        {
          "lang": "fr",
          "value": "Exemple d'Université"
        }
      ]
    },
    "id": "did:example:ebfeb1f712ebc6f1c276e12ec21"
  }```

@andorsk andorsk marked this pull request as ready for review July 4, 2022 05:01
@andorsk andorsk mentioned this pull request Jul 4, 2022
Copy link
Member

@decentralgabe decentralgabe left a comment

Choose a reason for hiding this comment

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

@andorsk
before moving your examples, let's agree on whether they should be tests or as is.

the main benefit of tests:

  • not included in the binary when you're using the sdk as a dependency
  • simple to run independently
  • can easily determine if the examples break, as tests are run for a CI-precondition
  • simpler error handling with assertions

benefits of main.go:

  • easier to see "real world" usage

what do you think?

@andorsk
Copy link
Contributor Author

andorsk commented Jul 6, 2022

Me personally, I've suggested we follow precedence and standards, which is that these are not tests (b/c they aren't) and run as example files. There are more reasons I can give, but ultimately I don't think we are facing a unique problem here. It seems to me, how things have been done historically seems like a reasonable way to handle it.

See here for some examples of examples 😆 :
https://github.com/golang-standards/project-layout/tree/master/examples
https://github.com/cossacklabs/themis/tree/master/docs/examples/go

That being said: I'd suggest three lines here of action, which would result in a conclusion:

  1. We can agree based upon the evidence above. But given we've both highlighted reasons and have not changed positions, it seems like it's at an impasse right now.
  2. We can ask some third person to weigh in and tie break this.
  3. If you feel strongly about this, I can change them to tests. As I mentioned before, It's more important we have something in for now. There are things I'm going to want to dig my heel in potentially later, but this isn't one of them and I'm willing to budge on this if that's going to move things forward.

What's the most important here is we have some examples available for everyone else. We can always change things later if we realize we made a mistake here.

Copy link
Member

@decentralgabe decentralgabe left a comment

Choose a reason for hiding this comment

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

@andorsk I appreciate the links, I wasn't aware this was a convention.

I'm OK to merge as-is. In the future, we can consider creating a new repo ssi-sdk-examples and moving them there; however for now this is an excellent start.

I noticed other examples rely on handleError, perhaps worth creating an examples util file or something similar.

@decentralgabe decentralgabe merged commit bd77e66 into TBD54566975:main Jul 6, 2022
@andorsk
Copy link
Contributor Author

andorsk commented Jul 6, 2022

Thanks @decentralgabe for merging this in.

And yep! I think that's totally fair. In fact, maybe we should see about putting all the examples in the same directory. Rather than different ones.

Lots of work to do on the examples! I'm happy to chat about scope for improving them.

@andorsk andorsk deleted the vc-example branch July 6, 2022 18:23
decentralgabe added a commit that referenced this pull request Jul 6, 2022
* main:
  did example. (#127)
  added verifiable credential example (#128)
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.

None yet

2 participants