-
Notifications
You must be signed in to change notification settings - Fork 72
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
feat: player next stream #536
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this PR remove the need to go through the MetaDetails model before loading the next (suggested) stream (i.e. #527) ?
Basically the same suggested stream should be returned from the Player itself I suppose.
This leads me to another question: What about the StreamsBucket? Shouldn't we update the Player model to also takes care of adding the Stream to the StreamsBucket?
Right now on Player::Load
it adds the selected stream to the StreamBucket but I'm not sure if we should do this beforehand when selecting next_stream
.
src/unit_tests/player/next_stream.rs
Outdated
} | ||
|
||
fn fetch_handler(request: Request) -> TryEnvFuture<Box<dyn Any + Send>> { | ||
println!("{:?}", request.url); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can remove this println
I'm not sure, we should discuss it
Right, either we return the
I'm not sure if it's an issue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Let's discuss this with @TheBeastLT in Slack and see if the other applications that go through the MetaDetails first to keep the history navigation correct should remain or we should use the next_stream
of this PR
Related #397
|
No description provided.