Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[2.8][Filesystem] Changed dumpFile to allow dumping to streams...#14970
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
…tive tempnam() and fixed dumpFile to allow dumping to streams
markchalloner commentedJul 13, 2015
Hi@xabbuh, Are there any further actions needed on this before it can be merged? Thanks |
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.
I think it would be better to throw an exception if an error occurred instead of silently fail and return a magic return value (same below).
fabpot commentedOct 6, 2015
@markchalloner Are you still interested in finishing this PR? |
markchalloner commentedOct 6, 2015
@fabpot. Sure |
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 removed as we only document "major" features in READMEs.
markchalloner commentedOct 6, 2015
@fabpot Thanks for the feedback. On reflection I am happy to pass this over to anyone who wants to pick this up, unfortunately I don't currently have the time to dedicate to this. |
fabpot commentedOct 6, 2015
Thanks for your feedback@markchalloner I'm closing this PR. Someone can take over anytime by finishing this PR. |
…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
...as rename can only work where $oldname and $newname have the same streams wrappers.
Added tempnam, stream wrapper aware version of tempnam(), to do the heavy lifting.