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

refactor changeStream nested promise chain #58

Merged

Conversation

alexmaras
Copy link
Contributor

Refactored the changeStream function to clean up a nested promise chain and make it more readable.
No logic changes have occurred here - the internal function has been made async in order to handle the promises in a more linear fashion and avoid the new Promise/resolve() pattern and still maintain order of operations

@YouKnowBlom
Copy link
Contributor

Awesome! If it is one thing this client needs it's definitely refactoring. Will take a look ASAP.

@alexmaras
Copy link
Contributor Author

Great, thanks! I noticed #59 working towards some similar improvements - I'd be happy to help out with an effort to convert to some of those newer constructs (async/await, const etc)

Is there documentation on how best to test this codebase locally?

@YouKnowBlom
Copy link
Contributor

I'd be happy to help out with an effort to convert to some of those newer constructs (async/await, const etc)

Please do, that would be very much appreciated! :)

Is there documentation on how best to test this codebase locally?

There isn't currently a good way to test without a cast device unfortunately. I've looked into it a little bit and there seems to be ways to emulate a device with this and test that way, but I haven't had the time to look into it further.

@SteveDinn
Copy link

I'd like to help test this as well. I'm anxious to have a better chrommecast client, but I'm not sure how (or where) to apply any updates.

@neopc10
Copy link

neopc10 commented Sep 9, 2020

I'd like to help test this as well. I'm anxious to have a better chrommecast client, but I'm not sure how (or where) to apply any updates.

I think maincontroller.js is in need of Queueing support for the web receiver and then Cast Connect receiver support for the Android TV repo in preparation for the new devices due out.

The Android 2.0.0 repo is in need of a new Android sender, as I believe it is currently missing. If jellyfin-web's Google Cast ever gets separated away, I'm sure the Chrome sender would benefit from a one-over.

Copy link
Contributor

@YouKnowBlom YouKnowBlom left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, #60 blocked me when I got the time to test. Works perfectly from my testing, thanks for the PR :)

@YouKnowBlom YouKnowBlom merged commit 2f7a446 into jellyfin:master Sep 20, 2020
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.

4 participants