Change annotations fields in ResourceDecriptor from objects to values #251
Conversation
This change expands support from objects to all JSON values. This allows maximum flexibility for producers and better matches the implicit reading of the spec many, including the SLSA project, had. Fixes: in-toto#242 Signed-off-by: Joshua Lock <[email protected]>
There was a problem hiding this comment.
Thanks @joshuagl ! One change that would be help would be to update/add to the examples of the resource descriptor spec to show a more accurate use of value type fields.
Signed-off-by: Joshua Lock <[email protected]>
|
Whoops, I missed the example. I just pushed 48e03ef to change the existing example. I can try and add one or two more next week if that's desirable? |
|
Thanks! Yeah, I think it would be helpful to have a second example on there that shows different possible values, but I don't think it's crucial. The other thing to consider is how these changes will affect the language bindings/unit tests. But we can resolve that in a separate PR, I figure. |
I agree with the useful but not crucial sentiment here. Can we consider the PR as is? I can file an issue for more examples. |
I think so. Thanks! |
There was a problem hiding this comment.
LGTM. +1 with @marcelamelara comment about adding other examples that shows different possible values. Can be in another PR.
The changes in in-toto#251 to update the type of the annotation value in a resource descriptor was not reflected in the tests, causing them to fail. Update the tests to use the new annotation value type. Signed-off-by: Joshua Lock <[email protected]>
fix(tests): fix go tests since #251
This change expands support from objects to all JSON values. This allows maximum flexibility for producers and better matches the implicit reading of the spec many, including the SLSA project, had.
Fixes: #242