feat(bundle.go): add RequiredExtensions #83
Conversation
| Credentials map[string]Credential `json:"credentials,omitempty" mapstructure:"credentials"` | ||
| Outputs *OutputsDefinition `json:"outputs,omitempty" mapstructure:"outputs"` | ||
| Definitions definition.Definitions `json:"definitions,omitempty" mapstructure:"definitions"` | ||
| RequiredExtensions []string `json:"requiredExtensions,omitempty" mapstructure:"requiredExtensions"` |
There was a problem hiding this comment.
Since this slice can contain duplicate entries, should we add a test to cover that case and nail down what the behaviour should be?
There was a problem hiding this comment.
Ah, good point. Should we just check that there are no duplicates during validation? If there are duplicates, throw an error mentioning this?
There was a problem hiding this comment.
The slice can also contain items that are not present in the custom map, and we should probably validate that as well.
There was a problem hiding this comment.
Ah, good point. Should we just check that there are no duplicates during validation? If there are duplicates, throw an error mentioning this?
Yes, that's probably better than issuing a warning and proceeding as that would encourage duplication/sloppiness.
There was a problem hiding this comment.
Updated; Please take a look when convenient.
vdice
force-pushed
the
feat/req-ext
branch
2 times, most recently
from
913dd6c to
17e691c
Compare
There was a problem hiding this comment.
This looks great, a single mention to maybe also test the case when requiredExtensions contains an item that is not present in custom.
That being said, we should also expose how drivers advertise what custom extensions they can handle, and check before actually executing the action - should it also be part of this PR, or should we merge this sooner to be in sync with the bundle definition?
Good call; see unit test added -- it should cover scenario mentioned.
I would suggest we tackle this in a different ticket/PR. |
RequiredExtensionsstring array field to theBundlestruct, per the CNAB Spec (added in Adding "requiredExtensions" to core spec cnab-spec#220)Closes #71