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 issues/483 and add a basic Node example #484

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

Conversation

nicolaslevy
Copy link

Description

Issue

#483

Type

  • Breaking change
  • Enhancement
  • Fix
  • Documentation
  • Tooling

Copy link
Contributor

@ZacharieTFR ZacharieTFR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, thank you for your contribution, it is much appreciated! I've left a few comments

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not push the dist, we will do it during the next release 😉

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅 done in f910ea0

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh... my bad, I deleted them :P Let me restore the original dist folder

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap, now the dist folder has no changes. Solved in commit 2b48c61

examples/node/package.json Outdated Show resolved Hide resolved
nicolaslevy and others added 2 commits December 17, 2024 18:02
Example Node: re-use the same information than the root package

Co-authored-by: Zac  <[email protected]>
@nicolaslevy
Copy link
Author

Thank you @ZacharieTFR, I made those changes

Copy link
Contributor

@ZacharieTFR ZacharieTFR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you moved the example folder and added the node, can you update the readme aswell ?

To explore a practical example of how the vast client can be implemented, please visit this [link](https://githubbox.com/dailymotion/vast-client-js/tree/master/examples).

examples/node/index.js Outdated Show resolved Hide resolved
@nicolaslevy
Copy link
Author

As you moved the example folder and added the node, can you update the readme aswell ?

To explore a practical example of how the vast client can be implemented, please visit this [link](https://githubbox.com/dailymotion/vast-client-js/tree/master/examples).

I changed this text in the last commit:

To explore a practical example of how the VAST client can be implemented in web applications please visit this link. Additionally, use this link to learn how to use the VAST client in a basic Node application.

@ZacharieTFR Let me know what you think.

Copy link
Contributor

@ZacharieTFR ZacharieTFR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM !

@nicolaslevy
Copy link
Author

Awesome!! just FYI: we are using this library in this project https://github.com/montevideo-tech/vast-2-sgai/ .A basic VAST-2-SGAI (VAST to Asset List and ListMPD) opens-ource microservice :)

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

Successfully merging this pull request may close these issues.

2 participants