Fix/dynamically added elements #198
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bblaszkow06
force-pushed
the
fix/dynamically-added-elements
branch
from
85236bd to
66d3144
Compare
Hajto
approved these changes
Sep 26, 2019
| @@ -125,6 +128,8 @@ defmodule Membrane.Core.PlaybackHandler do | |||
| end | |||
| end | |||
|
|
|||
| @spec unlock_target_state(pb) :: {:ok | {:error, :playback_already_unlocked}, pb} | |||
| when pb: Playbackable.t() | |||
| def unlock_target_state(playbackable) do | |||
There was a problem hiding this comment.
This case is not tested (at least via ExUnit), maybe it should? (soft suggestion)
There was a problem hiding this comment.
This might be done, but in separate PR I think
| @@ -73,6 +73,7 @@ defmodule Membrane.Core.PlaybackHandler do | |||
| def state_change_callback(:prepared, :playing), do: :handle_prepared_to_playing | |||
| def state_change_callback(:prepared, :stopped), do: :handle_prepared_to_stopped | |||
|
|
|||
| @spec change_playback_state(PlaybackState.t(), module(), Playbackable.t()) :: handler_return_t() | |||
There was a problem hiding this comment.
handler_return_t assumes that error can be any,
After a once over the file I found those as possible options:
- playback_already_unlocked
- invalid_current_playback_state
- invalid_target_playback_state
- target_and_current_states_equal
If error is not explicitly one those, maybe we could change handler_return_t and make the error reason at least an atom narrowing down the type?
There was a problem hiding this comment.
Don't see the point of it. handler_return_t is used as a result of a callback so the returned error can have any reason and it doesn't have to be an atom (a tuple may appear for example)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.