Skip to content
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

Fix/icon clicks tracking #40

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

danielzurawski
Copy link
Contributor

Hi Tom (once again, although 4 years later!),

  1. I've discovered that even though Icons (with click through and view event) are supported, they are not actually outputted in the final XML.

Here are the proposed changes to add the missing:

  • Icon/IconClicks/IconClickThrough
  • Icon/IconClicks/IconClickTracking
  • Icon/IconViewTracking
    in the generated output for a Linear Ad.
  1. Also, I had some issues getting libxmljs installed in node.js 11, turns out that the outdated libxmljs was no longer compiling. Updating it to the latest libxmljs fixes it.

  2. I also found that since February 2018, there is someone working on a fully IAB compliant VAST-XML builder library: https://github.com/DavidBabel/vast-builder - I haven't tried it yet, but it looks pretty decent. I wonder if perhaps deprecating this in favour of the vast-builder might be a way to go in the future?

@tbuchok
Copy link
Owner

tbuchok commented Jan 16, 2019

@danielzurawski nice to hear from you!

before evaluating the PR -- perhaps we discuss the deprecation suggestion in regard to https://github.com/DavidBabel/vast-builder.

does that lib get you exactly what you need, right now? if so, perhaps it's a good time to deprecate this lib and point people over to vast-builder.

alternatively, do you see any value in us keeping this project alive? i'm admittedly not working in the video space at the moment, so the IAB ecosystem is very much not in my headspace!

if you're unblocked and feel vast-builder is sufficient, i'm inclined to deprecate! 😢

@danielzurawski
Copy link
Contributor Author

@tbuchok Sorry for taking a long time to respond, been busy with things. I recommend people to at least evaluate vast-builder before using this library, but it's only got a few stars/not that many maintainers yet either.

The vast-builder library does indeed support everything that I need and I tried it with a proof-of-concept service. It's nicely written and I like how the different versions of VAST are defined in Yaml spec files and there are tests against them.
However, the service that we have deployed in production relies on your library (vast-xml) and will remain so until/if it gets deprecated or we re-factor it to use the aforementioned POC concept service (using the vast-builder)to support multiple VAST versions.

So in terms of keeping this project alive, I guess if anyone is in a similar boat and has it already deployed, maybe adding minor fixes / keeping it hosted makes sense, but having a reference to the other library (vast-builder)?
Also, When playing around with vast-builder, in it's "Project" page I found a mention of https://github.com/zentrick/iab-vast-model, which is another library, but I still prefer vast-builder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants