-
-
Notifications
You must be signed in to change notification settings - Fork 160
File object #62
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
File object #62
Conversation
I guess it makes sense to adopt PSR-7's naming convention? http://www.php-fig.org/psr/psr-7/#3-6-psr-http-message-uploadedfileinterface |
That makes sense, including method names? But I think actually implementing PSR-7 should be done in a/the next BC release |
@clue updated PR to make them resemble PSR-7 UploadedFIle |
Ping @clue |
|
||
use React\Stream\ReadableStreamInterface; | ||
|
||
class UploadedFile implements UploadedFileInterface |
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 this be @internal
?
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.
Yes 👍
This code looks good to me – except that it doesn't have any use case right now :-) IMO it makes sense to also introduce the use case in the same PR as this one would be rather pointless otherwise. |
Yes it is rather pointless with out a use case. But to avoid creating an enormous PR again I'm going to have a set of PR's depending on each. Those PR's contain small logical components. |
👍 LGTM |
Didn't mean to close this one, only cleaned up a (seemingly unrelated) feature branch. @WyriHaximus does it make sense to file this again or do you want until the milestone is ready? 👍 |
Superseded by #199 |
This is the first PR extracted from #41 and is solely for the
File
object used in the streaming body parser's. (See #41 (comment) for reference.)