Skip to content

File object - #62

Closed
WyriHaximus wants to merge 3 commits into
masterfrom
feature-file-object
Closed

File object#62
WyriHaximus wants to merge 3 commits into
masterfrom
feature-file-object

Conversation

@WyriHaximus

Copy link
Copy Markdown
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

clue commented Aug 30, 2016

Copy link
Copy Markdown
Member

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
Copy Markdown
MemberAuthor

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
Copy Markdown
MemberAuthor

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

@WyriHaximus

Copy link
Copy Markdown
MemberAuthor

Ping @clue

Comment threadsrc/UploadedFile.php

use React\Stream\ReadableStreamInterface;

class UploadedFile implements UploadedFileInterface

Copy link
Copy Markdown
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
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yes 👍

@clue

clue commented Sep 13, 2016

Copy link
Copy Markdown
Member

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.

@clueclue modified the milestone: v0.5Sep 13, 2016
@WyriHaximus

Copy link
Copy Markdown
MemberAuthor

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

jsor commented Oct 24, 2016

Copy link
Copy Markdown
Member

👍 LGTM

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

clue commented Feb 16, 2017

Copy link
Copy Markdown
Member

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 masterFebruary 16, 2017 14:45
@clueclue reopened this Feb 16, 2017
@clueclue modified the milestone: v0.8.0Feb 16, 2017
@WyriHaximus

Copy link
Copy Markdown
MemberAuthor

Superseded by #199

@clue
clue deleted the feature-file-object branch August 16, 2017 08:24
@clueclue removed this from the v0.8.0 milestone Aug 16, 2017
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@WyriHaximus@clue@jsor