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.

Sign up for GitHub

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

Jump to bottom

Expose Merge as a merge() function #237

Closed
llucax opened this issue Nov 9, 2023 · 0 comments · Fixed by #238
Closed

Expose Merge as a merge() function #237

llucax opened this issue Nov 9, 2023 · 0 comments · Fixed by #238
Assignees
@llucax
Labels
part:core Affects the core types (`Sender`, `Receiver`, exceptions, etc.) scope:breaking-change Breaking change, users will need to update their code type:enhancement New feature or enhancement visitble to users

Comments

@llucax
Copy link
Contributor

llucax commented Nov 9, 2023

What's needed?

There is not much use in having Merge as a class from a user point of view, users normally only care about merging some channels, so having a class just makes the syntax more weird for most use cases that will be async for msg in merge(...).

Proposed solution

Make Merge private (_Merge) and create a new function that just does return _Merge(...).

Use cases

No response

Alternatives and workarounds

No response

Additional context

No response
@llucax llucax added type:enhancement New feature or enhancement visitble to users part:core Affects the core types (`Sender`, `Receiver`, exceptions, etc.) scope:breaking-change Breaking change, users will need to update their code labels Nov 9, 2023
@llucax llucax self-assigned this Nov 9, 2023
This was referenced Nov 9, 2023
Open
Merged
github-merge-queue bot pushed a commit that referenced this issue Nov 16, 2023
@llucax
Replace Merge and MergeNamed with merge() (#238)
9266229 
- Remove `MergeNamed`
- Add a `merge()` wrapper function that replaces `Merge`
- Rename `Merge` to `_Merge` to make it private more explicitly
- Rename `_Merge` variable `args` to `receivers`
- Improve string representation of merge receiver
- Raise an exception is `merge()` is called with less than two receivers

Fixes #236, fixes #237.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@llucax llucax

Labels
part:core Affects the core types (`Sender`, `Receiver`, exceptions, etc.) scope:breaking-change Breaking change, users will need to update their code type:enhancement New feature or enhancement visitble to users
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Replace Merge and MergeNamed with merge() llucax/frequenz-channels-python
1 participant
@llucax