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

api: Audience#foreach and Audience#filter #365

Merged
merged 4 commits into from
Sep 5, 2021
Merged

Conversation

kashike
Copy link
Member

@kashike kashike commented May 13, 2021

No description provided.

Copy link
Member

@kezz kezz left a comment

Choose a reason for hiding this comment

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

I think this is a decent idea - there's a lot of good use cases for it at least. However, I wonder if it might just be worth implementing Iterable at this point?

@kezz
Copy link
Member

kezz commented May 13, 2021

I mentioned this in Discord and it got lost to time so I'll just repeat here. It could be a good idea to just provide a method to get the audience as an iterable directly. Maybe exposing a stream as well could be a more "standard" way of implementing the filter system.

@zml2008 zml2008 mentioned this pull request May 31, 2021
@lucko
Copy link
Member

lucko commented Jun 4, 2021

To be honest, I don't see the usecase...

foreach: why iterate when all of the methods on the audience delegate anyway?
filter: can appreciate how this might be useful, but what properties do you have to filter by? Audience doesn't really give you much unless you try casting etc...

Copy link
Member

@kezz kezz left a comment

Choose a reason for hiding this comment

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

Needs updating in regards to @since tags and style changes, but I like the concept - especially so with the addition of pointers.

@zml2008 zml2008 added this to the 4.9.0 milestone Aug 2, 2021
@zml2008 zml2008 linked an issue Sep 4, 2021 that may be closed by this pull request
@kezz kezz linked an issue Sep 4, 2021 that may be closed by this pull request
This pull request was closed.
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.

Why are methods annotated with @OverrideOnly? Add Audience#filter method
6 participants