-
Notifications
You must be signed in to change notification settings - Fork 213
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
[client] Send warning event on missing required fields #307
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.
Great job! Let me know what you think about my comments :)
* @return {Ad} | ||
*/ | ||
export function parseAd(adElement) { | ||
export function parseAd(adElement, emit) { |
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.
Because of the new parameter this PR should be flagged as Breaking change
as it changes api
…est for vast verification
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.
LGTM! 🎉
Lets wait for @rumesh to take a look as well before merging :)
src/parser/parser_verification.js
Outdated
{ name, parentName, attributes, subElements, oneOfResources }, | ||
emit | ||
) { | ||
let message = `Element `; |
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.
Might be better to put the name here already and use else if, and else for the last statement, if we can only have one kind of error per message.
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.
Just one smol comment otherwise it's all good for me ;)
Description
Trigger a "VAST-warning" if any required attributes or subelements are missing in any element.
According to https://iabtechlab.com/wp-content/uploads/2018/11/VAST4.1-final-Nov-8-2018.pdf
Type