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

Allow sinks/sources to have multiple inputs/outputs #1088

Merged
merged 3 commits into from
Jul 4, 2024

Conversation

p-snft
Copy link
Member

@p-snft p-snft commented Jul 2, 2024

There was a warning and a comment that having multiple inputs/outputs for one Sink/Source is unintended but should be possible. I think we can remove that "strong recommendation" as it just works.

To not let beginners fall into the trap of having a dangling Sink or Source, like the old warning also might have prevented, inputs and outputs are now explicitly named arguments without a default.

There was a warning and a comment that having multiple inputs/outputs
for one Sink/Source is unintended but should be possible.
I think we can remove that "strong reccomendation" as it just works.

To not let beginners fall into the trap of having a dangling Sink
or Source, like the old warning also might have prevented, inputs and
outputs are now explicitly named arguments without a default.
@p-snft p-snft self-assigned this Jul 2, 2024
@p-snft p-snft added this to the v0.5.x milestone Jul 2, 2024
@p-snft
Copy link
Member Author

p-snft commented Jul 2, 2024

Coverage decreases as a tested (anti-) feature is removed.

@p-snft p-snft requested a review from a team July 2, 2024 12:44
@p-snft
Copy link
Member Author

p-snft commented Jul 4, 2024

@uvchik gave positive feedback on another channel and suggested to have tests to explicitly make this part of the API.

(Tests work but reporting to Coveralls fails for some tests.)

@p-snft p-snft merged commit a448f7f into dev Jul 4, 2024
13 of 14 checks passed
@p-snft p-snft deleted the feature/multi_input_sink branch July 4, 2024 09:56
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.

1 participant