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

Crash groups in bins #521

Merged
merged 57 commits into from
Feb 16, 2023
Merged

Crash groups in bins #521

merged 57 commits into from
Feb 16, 2023

Conversation

FelonEkonom
Copy link
Member

@FelonEkonom FelonEkonom marked this pull request as ready for review February 1, 2023 11:03
@FelonEkonom FelonEkonom requested a review from mat-hek as a code owner February 1, 2023 11:03
@FelonEkonom FelonEkonom requested a review from varsill February 1, 2023 11:03
@mat-hek mat-hek changed the base branch from master to handle-child-pad-removed-bug-fix February 15, 2023 11:58
@FelonEkonom FelonEkonom force-pushed the handle-child-pad-removed-bug-fix branch from fc820bf to 515984d Compare February 15, 2023 13:22
@FelonEkonom FelonEkonom force-pushed the handle-child-pad-removed-bug-fix branch from 515984d to ad72e26 Compare February 15, 2023 13:33
@@ -318,7 +318,7 @@ defmodule Membrane.ChildrenSpec do

@type children_spec_options :: [
group: Child.group(),
crash_group_mode: Membrane.CrashGroup.mode() | nil,
crash_group_mode: Membrane.CrashGroup.mode(),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
crash_group_mode: Membrane.CrashGroup.mode(),
crash_group_mode: :temporary | nil,

and we can remove Membrane.CrashGroup altogether.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is used in 2 other places as well, so I can remove this CrashGroup module and move typespec to the another file, but I would still have a typespec describing :temporary | nil

Base automatically changed from handle-child-pad-removed-bug-fix to master February 16, 2023 10:59
@FelonEkonom FelonEkonom requested a review from mat-hek February 16, 2023 13:28
@FelonEkonom FelonEkonom removed the request for review from varsill February 16, 2023 13:46
…nore :child_pad_removed from terminating children
Membrane.Pipeline.behaviour_info(:callbacks)
Membrane.Bin.behaviour_info(:callbacks)
Copy link
Member

Choose a reason for hiding this comment

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

?

@FelonEkonom FelonEkonom requested a review from mat-hek February 16, 2023 15:09
@FelonEkonom FelonEkonom merged commit 45c537e into master Feb 16, 2023
@FelonEkonom FelonEkonom deleted the crash-groups-in-bins branch February 16, 2023 15:13
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.

2 participants