Skip to content

Conversation

@tjenkinson
Copy link
Contributor

@tjenkinson tjenkinson commented Jul 27, 2017

The maestro player api is different, but player-api.js is an adaptor so that we can keep the current api for now. Maybe in the future we will do a major release and make maestro's api public

@tjenkinson tjenkinson requested a review from a team July 27, 2017 16:29
@tjenkinson tjenkinson force-pushed the update-playback-library branch 2 times, most recently from 7196c49 to 9d3d799 Compare July 27, 2017 16:44
src/stream.js Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

var -> let

@janmonschke
Copy link
Contributor

What is the impact on the file size?

@tjenkinson
Copy link
Contributor Author

@janmonschke
current: 233KB
new (with hls and html5 player): 286KB
new (with just html5 player): 180KB

We aren't serving hls on public api at the moment but we might do in the future

@janmonschke
Copy link
Contributor

Before minification?

@tjenkinson
Copy link
Contributor Author

no after

src/stream.js Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to use the api module for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was, but it doesn't work because it is a post request, and the api module wants to add the oauth token and also other things as form data, instead of query params.

@tjenkinson tjenkinson force-pushed the update-playback-library branch 2 times, most recently from 7219499 to f7f02df Compare July 28, 2017 09:55
@tjenkinson
Copy link
Contributor Author

This requires #82 because the current version of npm installs peer dependencies by default, and this means some unnecessary packages are included.

@tjenkinson tjenkinson force-pushed the update-playback-library branch from fdeacec to e2604c0 Compare August 16, 2017 14:55
@tjenkinson
Copy link
Contributor Author

@janmonschke you happy with this?

@tjenkinson tjenkinson force-pushed the update-playback-library branch from e2604c0 to c925b08 Compare August 17, 2017 11:17
@tjenkinson tjenkinson merged commit 596d70e into master Aug 17, 2017
@tjenkinson tjenkinson deleted the update-playback-library branch August 17, 2017 15:45
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