8000 [Filesystem] Replace remaing docblocks by type-hints by ro0NL · Pull Request #24926 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Filesystem] Replace remaing docblocks by type-hints #24926

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

Closed
wants to merge 1 commit into from
Closed

[Filesystem] Replace remaing docblocks by type-hints #24926

wants to merge 1 commit into from

Conversation

ro0NL
Copy link
Contributor
@ro0NL ro0NL commented Nov 11, 2017
Q A
Branch? master
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #...
License MIT
Doc PR symfony/symfony-docs#...

Partially forgotten in #24722 as only private getSchemeAndHierarchy was updated.

Adds simplification of toIterator.

edit: wrong labels.

* @return \Traversable
*/
private function toIterator($files)
private function toIterable($files): iterable
Copy link
Member

Choose a reason for hiding this comment

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

the name change is debatable, and creates useless diff lines, thus won't help merges
should be reverted IMHO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hence #24928 :)

}

return $files;
return is_iterable($files) ? $files : array($files);
Copy link
Member

Choose a reason for hiding this comment

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

this implementation is not compatible with the previous, won't it create BC breaks f the return is forwarded as return value of a public method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

private API :) checked all usages; they truly rely on a foreachable value.

@ro0NL ro0NL closed this Nov 13, 2017
@ro0NL ro0NL deleted the filesystem branch November 18, 2017 10:42
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.

3 participants
0