search

cnabio / cnab-go

A Go implementation of CNAB Core 1.0
MIT License
69 stars 37 forks source link

Add schemaVersion to credentialSet #229

Closed carolynvs closed 4 years ago

carolynvs commented 4 years ago

Add a schemaVersion field to credentialSet so that we can identify the spec version that the credential set adheres to. This was very useful for claims, we use it for data migrations when the claim spec changes, and it is best to have it before we need to change the spec. Worst case we never change the spec again and we all win. 😄

The last change to the credential-set spec was https://github.com/cnabio/cnab-spec/releases/tag/cnab-credentialsets-1.0.0-DRAFT+b6c701f

carolynvs commented 4 years ago

So one thing I'm unsure about is that @vdice recently added support for parameter sets. It's not in the spec, but it would be great to see that added to it and cnab-go. If we did that, would the spec still be called credentialsets? They have different schema, the list of parameters goes under a field named parameters in a parameter set and in a credential set it has a field named credentials. Or would we have two specs? 🤷‍♀️

vdice commented 4 years ago

Good questions, @carolynvs .

I also noticed that an actual schema for a credentialset isn't yet present in the cnab-spec repo, beyond the in-line example/text in https://github.com/cnabio/cnab-spec/blob/master/802-credential-sets.md. We can perhaps create an issue in the spec repo to create one.

I'm conflicted on whether or not to try to combine parameter set and credential set into one 'valuesource' schema or leave them separate. The latter approach perhaps has benefits in comprehension and, theoretically, flexibility in the future, if, for example, a parameter set adds or removes sources that a credential set lacks or wishes to keep...

I'll create an issue in this repo around the general area of adding in parameter set definitions/logic and include the question/task around pinning to a correlating schema and/or spec tag.

carolynvs commented 4 years ago

@vdice What do you think of this alternate pattern for getting at the DefaultSchemaVersion so that we don't have to check for parse errors at runtime?

https://github.com/cnabio/cnab-go/pull/229/files#diff-2c1fa05be1e11946f6192fea4d31108dR20

vdice commented 4 years ago

@carolynvs Sorry for the late reply! Somehow missed the last comment. 👍 I love the alternate approach you've gone with -- great idea!

carolynvs commented 4 years ago

@radu-matei What do you think? Right now I'm writing Porter to work with 2 specs, since that is how CNAB is written today.