-
Notifications
You must be signed in to change notification settings - Fork 6
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
Leverage Conviva types #106
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice update! A few comments though
CHANGELOG.md
Outdated
### Deprecated | ||
- Removed `framework` and `frameworkVersion` custom metadata fields (custom tags) | ||
- Removed `CustomContentMetadata`, use `Metadata['custom']` instead |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't these actually be in a ### Removed
section?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in cce84d2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for L14, or not? We missed that there but would be good to clean up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, done here ecc72d4. 🙂
@convivainc/conviva-js-coresdk
as a peer dependency and reference types in the code explicitlyConviva.ContentMetadata
as a basis instead custom re-declaration of the wholeMetadata
build
more explicitly follow the types (just split it into methods that match types semantically)The above is done to reduce confusion and make future extensions easier to follow.