Skip to content

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

Closed
wants to merge 3 commits into from
Closed

File object #62

wants to merge 3 commits into from

Conversation

WyriHaximus
Copy link
Member

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.)

@clue
Copy link
Member

clue commented Aug 30, 2016

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

@WyriHaximus
Copy link
Member Author

That makes sense, including method names? But I think actually implementing PSR-7 should be done in a/the next BC release

@WyriHaximus
Copy link
Member Author

@clue updated PR to make them resemble PSR-7 UploadedFIle

@WyriHaximus
Copy link
Member Author

Ping @clue


use React\Stream\ReadableStreamInterface;

class UploadedFile implements UploadedFileInterface
Copy link
Member

Choose a reason for hiding this comment

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

Should this be @internal?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes 👍

@clue
Copy link
Member

clue commented Sep 13, 2016

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.

@clue clue modified the milestone: v0.5 Sep 13, 2016
@WyriHaximus
Copy link
Member Author

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.

@jsor
Copy link
Member

jsor commented Oct 24, 2016

👍 LGTM

@WyriHaximus WyriHaximus self-assigned this Nov 30, 2016
@WyriHaximus WyriHaximus changed the base branch from master to 0.5 November 30, 2016 20:34
@clue clue modified the milestone: v0.5.0 Feb 14, 2017
@clue clue closed this Feb 16, 2017
@clue
Copy link
Member

clue commented Feb 16, 2017

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? 👍

@clue clue changed the base branch from 0.5 to master February 16, 2017 14:45
@clue clue reopened this Feb 16, 2017
@clue clue modified the milestone: v0.8.0 Feb 16, 2017
@WyriHaximus
Copy link
Member Author

Superseded by #199

@clue clue deleted the feature-file-object branch August 16, 2017 08:24
@clue clue removed this from the v0.8.0 milestone Aug 16, 2017
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