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

Preventing internal testing notifications from reaching pipeline module #350

Merged
merged 4 commits into from
Nov 16, 2021

Conversation

daniel-jodlos
Copy link
Contributor

Changes from this Pull Request will make sure that notifications sent internally by different elements of Membrane.Testing will not be reaching the pipeline specified with module option

All notifications sent by Testing elements to Testing.Pipeline are now wrapped in Membrane.Testing.Notification struct. Previous implementation made it impossible to easily differentiate between user-specified notifications and those used by Testing module and introduced risk of conflicts

@@ -41,6 +43,12 @@ defmodule Membrane.Testing.Sink do
"""
]

defmacrop notify(payload) do
Copy link
Member

Choose a reason for hiding this comment

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

Why not defp?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Friday afternoon commits, I think that is the only explanation. Changed to defp

Copy link
Member

Choose a reason for hiding this comment

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

So you can move it to the end of the module too ;)

@mat-hek mat-hek merged commit 23cbbbf into master Nov 16, 2021
@mat-hek mat-hek deleted the filter-out-testing-notifications branch November 16, 2021 10:40
@mat-hek mat-hek added this to the core 0.9.0 milestone Nov 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants