Remote controlled pipeline #366
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
andpodob
force-pushed
the
remote-controlled-pipeline
branch
3 times, most recently
from
e288831 to
977dd83
Compare
andpodob
force-pushed
the
remote-controlled-pipeline
branch
3 times, most recently
from
870aef0 to
e6c2087
Compare
andpodob
force-pushed
the
remote-controlled-pipeline
branch
from
e6c2087 to
1ac37c9
Compare
mat-hek
requested changes
Dec 28, 2021
| @impl true | ||
| def handle_init(opts) do | ||
| %{controller_pid: controller_pid} = opts | ||
|
|
There was a problem hiding this comment.
Is there any logic behind these blank lines or do you just insert them in random places? :P
There was a problem hiding this comment.
I wanted this line to be right before the returned value :P
Comment on lines
40
to
41
| args = [__MODULE__, %{controller_pid: self()}, process_options] | ||
| apply(Pipeline, :start_link, args) |
There was a problem hiding this comment.
Suggested change
| args = [__MODULE__, %{controller_pid: self()}, process_options] | |
| apply(Pipeline, :start_link, args) | |
| Pipeline.start_link(__MODULE__, %{controller_pid: self()}, process_options) |
Comment on lines
49
to
50
| args = [__MODULE__, %{controller_pid: self()}, process_options] | ||
| apply(Pipeline, :start, args) |
There was a problem hiding this comment.
Suggested change
| args = [__MODULE__, %{controller_pid: self()}, process_options] | |
| apply(Pipeline, :start, args) | |
| Pipeline.start(__MODULE__, %{controller_pid: self()}, process_options) |
| It is required to firstly use the `Membrane.RemoteControlled.Pipeline.subscribe/2` before awaiting | ||
| any events. `pattern` has to be a match pattern. | ||
| """ | ||
| defmacro await(pattern) do |
There was a problem hiding this comment.
this should accept a pipeline pid as the first argument, as subscribe does
|
|
||
| @doc """ | ||
| Subscribes to a given `subscription_pattern`. | ||
| The `subscription_pattern` must be a match pattern. |
There was a problem hiding this comment.
the docs should mention event_t
|
|
||
| alias Membrane.Pipeline | ||
|
|
||
| @type event_t :: |
There was a problem hiding this comment.
it's a bit ambiguous with the Membrane.Event, maybe just message_t ?
…. Split await macro into multiple functions
…ide documentation and typespecs
…use await_generic macro
mat-hek
requested changes
Mar 23, 2022
mat-hek
requested changes
Mar 24, 2022
…ngful variable names in the subscription matcher. Add moduledoc for the abstract RemoteControlled.Message module
…aneframework/membrane_core into remote-controlled-pipeline
mat-hek
previously approved these changes
Mar 28, 2022
rename `message_t` typespec into `t` Co-authored-by: Mateusz Front <mateusz.front@swmansion.com>
mat-hek
previously approved these changes
Mar 30, 2022
…air an markdown generator error
mat-hek
requested changes
Mar 30, 2022
… code snippets of the remote controlled pipeline documentation
mat-hek
approved these changes
Mar 31, 2022
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.
closes #336