Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Filesystem] Changed dumpFile to allow dumping to streams#16156
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
aa428cc toa5b16abCompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
should be removed and\RuntimeException used in the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Even better, as we have exception classes for this component, they should be used instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
should be remove and\Phar should be used in the code.
pierredup commentedOct 12, 2015
Status: Needs Review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Two whitespaces.
fabpot commentedOct 13, 2015
👍 |
jakzal commentedOct 15, 2015
👍 status: reviewed |
jakzal commentedOct 15, 2015
@pierredup I was too quick. |
fabpot commentedOct 16, 2015
@pierredup Can you have a look at the Windows failure? Thanks. |
…tive tempnam() and fixed dumpFile to allow dumping to streams
pierredup commentedNov 9, 2015
ping@fabpot |
fabpot commentedNov 9, 2015
Thank you@pierredup. |
…ams (markchalloner, pierredup)This PR was merged into the 2.8 branch.Discussion----------[Filesystem] Changed dumpFile to allow dumping to streams| Q | A| ------------- | ---| Bug fix? | yes| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#10018| License | MIT| Doc PR |symfony/symfony-docs#5393This is a follow-up of#14970 with comments addressedCommits-------5ca7dee Fix modea17aa5e Fixed failing test for HHVM61a3afd Removed unused logic in MockStream247266c Update coding standard for MockStreamc6a7747 [Filesystem] added tempnam() stream wrapper aware version of PHP's native tempnam() and fixed dumpFile to allow dumping to streams
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
couldnt this be simlified to
return2 ===count($components) ?$components :array(null,$components[0]);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
seems so
This is a follow-up of#14970 with comments addressed