Skip to content

[Finder] mark the LazyIterator class as internal #40163

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
Merged
merged 1 commit into from
Feb 12, 2021
Merged

[Finder] mark the LazyIterator class as internal #40163

merged 1 commit into from
Feb 12, 2021
+2 −0

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Feb 12, 2021 •
edited
Loading

Q A
Branch? 4.4
Bug fix? no
New feature? no
Deprecations? no
Tickets #40040 (comment)
License MIT
Doc PR

@xabbuh xabbuh added this to the 4.4 milestone Feb 12, 2021
@xabbuh xabbuh mentioned this pull request Feb 12, 2021
Merged
@jderusse
Copy link
Member

Thanks for taking care of this. Could you please also take #40040 (comment) into account?

@ro0NL
Copy link
Contributor

ro0NL commented Feb 12, 2021 •
edited
Loading

why not keep support for Finder::append(new LazyIterator($callable))?

appendFromCallable for 5.x maybe, then it can become internal IMHO

@stof
Copy link
Member

stof commented Feb 12, 2021

@ro0NL this can was merged 4 hours ago. so there is no support to keep. Finder::append(new LazyIterator($callable)) is not supported in any released version of Symfony, because LazyIterator does not exist there.

@ro0NL
Copy link
Contributor

ro0NL commented Feb 12, 2021 •
edited
Loading

right, i meant introducing the feature in 4.x as a bonus side effect yes

unless we have other plans for 5.x, or we conclude it's not useful for users at all

@xabbuh xabbuh mentioned this pull request Feb 12, 2021
Merged
@fabpot
Copy link
Member

fabpot commented Feb 12, 2021

Thank you @xabbuh.

@fabpot fabpot merged commit 4626100 into symfony:4.4 Feb 12, 2021
@xabbuh xabbuh deleted the lazy-iterator-internal branch February 12, 2021 15:35
@xabbuh
Copy link
Member Author

xabbuh commented Feb 12, 2021

@jderusse I didn't read your comment before the PR was merged. Feel free to open a PR with your suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

Assignees
No one assigned
Labels
Finder Status: Reviewed
Projects
None yet
Milestone
4.4
Development

Successfully merging this pull request may close these issues.
6 participants
@xabbuh @jderusse @ro0NL @stof @fabpot @carsonbot