Skip to content

Fix issues/483 and add a basic Node example#484

Merged
clarissedmn merged 8 commits intodailymotion:masterfrom
montevideo-tech:master
Jan 30, 2025
Merged

Fix issues/483 and add a basic Node example#484
clarissedmn merged 8 commits intodailymotion:masterfrom
montevideo-tech:master

Conversation

@nicolaslevy
Copy link
Copy Markdown
Contributor

Description

Issue

#483

Type

  • Breaking change
  • Enhancement
  • Fix
  • Documentation
  • Tooling

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor 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
Copy Markdown
Contributor 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
Copy Markdown
Contributor 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

Comment thread examples/node/package.json Outdated
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  <ztaifour@gmail.com>
@nicolaslevy
Copy link
Copy Markdown
Contributor Author

Thank you @ZacharieTFR, I made those changes

Copy link
Copy Markdown
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).

Comment thread examples/node/index.js Outdated
nicolaslevy and others added 2 commits December 18, 2024 10:42
Fix: The EOF was missing

Co-authored-by: Zac  <ztaifour@gmail.com>
@nicolaslevy
Copy link
Copy Markdown
Contributor 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
Copy Markdown
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
Copy Markdown
Contributor 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 :)

@clarissedmn clarissedmn merged commit acddb39 into dailymotion:master Jan 30, 2025
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.

3 participants