Skip to content

Use Adapter *Contents instead in File Node - #80

Merged
WyriHaximus merged 1 commit into
masterfrom
unknown repository
Apr 26, 2020
Merged

Use Adapter *Contents instead in File Node#80
WyriHaximus merged 1 commit into
masterfrom
unknown repository

Conversation

@ghost

@ghost ghost commented Apr 26, 2020

Copy link
Copy Markdown

This patch updates Node/File to use the methods on the AdapterInterface instead of open the file for reading or writing and buffer the stream.

Fixes #63.

@ghost

ghost commented Apr 26, 2020

Copy link
Copy Markdown
Author

@WyriHaximus Please restart the CI build. However the Directory Test failures are unrelated to this PR.

@WyriHaximus

Copy link
Copy Markdown
Member

@CharlotteDunois will kick them for a while, need to finish #78 to resolve this

@WyriHaximus WyriHaximus added this to the v0.2.0 milestone Apr 26, 2020
@WyriHaximus

Copy link
Copy Markdown
Member

@CharlotteDunois all failing tests look EIO related to me

@clue clue left a comment

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.

Changes LGTM! Is (can) this also be covered by some functional tests?

@WyriHaximus
WyriHaximus self-requested a review April 26, 2020 13:59
@WyriHaximus
WyriHaximus requested a review from jsor April 26, 2020 13:59
@ghost

ghost commented Apr 26, 2020

Copy link
Copy Markdown
Author

@clue This is already covered by the Adapters/FileTest.php test suite. Unless you mean overwriting an existing file, that would need an additional test.

@WyriHaximus
WyriHaximus merged commit 02d8953 into reactphp:master Apr 26, 2020
@ghost
ghost deleted the file-node branch April 26, 2020 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

3 participants