Skip to content

Rewrite - #97

Merged
clue merged 1 commit into
reactphp:masterfrom
WyriHaximus-labs:rewrite
Feb 9, 2022
Merged

Rewrite#97
clue merged 1 commit into
reactphp:masterfrom
WyriHaximus-labs:rewrite

Conversation

@WyriHaximus

@WyriHaximusWyriHaximus commented Oct 6, 2021•
edited
Loading

Copy link
Copy Markdown
Member

This is a full rewrite of this package including the following features/changes:

  • ext-uv adapter
  • child process adapter
  • fallback (blocking) adapter (for native Windows)
  • simplified the internals to remove unnecessary complexity
  • file_get_contents/file_put_contents like API to read/write from/to files
  • start has become an object instead of an array

This PR doesn't include the following features/changes as those will be added in with followup PR's:

  • ext-eio adapter
  • macos support
  • mode support to define permissions when creating files/directories
  • symlinks read/creating/deleting
  • deleting of directories

cc: @SimonFrings

@WyriHaximusWyriHaximus added this to the v0.2.0 milestone Oct 6, 2021
@WyriHaximus
WyriHaximus requested a review from clueOctober 6, 2021 06:22
iorsa
iorsa reviewed Oct 6, 2021
View reviewed changes
Comment threadsrc/Uv/File.php Outdated
@WyriHaximus
WyriHaximusforce-pushed the rewrite branch 2 times, most recently from 9e4fc37 to 7be39ffCompareOctober 14, 2021 07:17
iorsa
iorsa reviewed Oct 30, 2021
View reviewed changes
Comment threadsrc/ModeTypeDetector.php
iorsa
iorsa reviewed Oct 30, 2021
View reviewed changes
Comment threadsrc/ModeTypeDetector.php
@WyriHaximus
WyriHaximusforce-pushed the rewrite branch 2 times, most recently from 5e6f62c to 6eb3c14CompareNovember 10, 2021 15:56
@WyriHaximus
WyriHaximusforce-pushed the rewrite branch 5 times, most recently from 7a1615c to e34ce17CompareFebruary 8, 2022 16:53
@WyriHaximus

WyriHaximus commented Feb 8, 2022

Copy link
Copy Markdown
MemberAuthor

@clue@SimonFrings This PR is now fully ready for review. The green build can be found here. And a readable render of the readme here.

skydiablo
skydiablo reviewed Feb 9, 2022
View reviewed changes
Comment threadREADME.md Outdated
This was referenced Feb 9, 2022
Closed
Closed
SimonFrings
SimonFrings reviewed Feb 9, 2022
View reviewed changes

@SimonFringsSimonFrings 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.

I looked over everything and focused on writing and syntax because functional adjustments can always be done in following PRs. If the tests are green, then I am happy ^^

Still some work to do but this is definitely a better version than before, nice work @WyriHaximus 👍

Comment threadREADME.md Outdated
Comment threadREADME.md Outdated
Comment threadsrc/Uv/Poll.php Outdated
Comment threadsrc/Uv/Poll.php Outdated
@WyriHaximus

WyriHaximus commented Feb 9, 2022

Copy link
Copy Markdown
MemberAuthor

@SimonFrings Thanks! Just updated this PR with your suggestions 👍

@WyriHaximus
Rewrite
016ceaa
This is a full rewrite of this package including the following features/changes:
* ext-uv adapter
* child process adapter
* fallback (blocking) adapter (for native Windows)
* simplified the internals to remove unnecessary complexity
* file_get_contents/file_put_contents like API to read/write from/to files
* start has become an object instead of an array
This PR doesn't include the following features/changes as those will be added in with followup PR's:
* ext-eio adapter
* macos support
* mode support to define permissions when creating files/directories
* symlinks read/creating/deleting
* deleting of directories
clue
clue approved these changes Feb 9, 2022
View reviewed changes

@clueclue 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.

@WyriHaximus Let's get this shipped! :shipit:

I agree with everything that's been said, so I only briefly eyeballed this. This may not be perfect (yet), but it's definitely a major improvement over the current experimental filesystem implementation. Let's get this in as-is and build on top of this with future PRs 👍

@clue
clue merged commit 5fae489 into reactphp:masterFeb 9, 2022
@WyriHaximus
WyriHaximus deleted the rewrite branch February 9, 2022 14:39
@WyriHaximus

WyriHaximus commented Feb 9, 2022

Copy link
Copy Markdown
MemberAuthor

@clue 🚢 🇮🇹 !

I agree with everything that's been said, so I only briefly eyeballed this. This may not be perfect (yet), but it's definitely a major improvement over the current experimental filesystem implementation. Let's get this in as-is and build on top of this with future PRs +1

Already on it! #99

@SimonFringsSimonFrings mentioned this pull request Feb 15, 2022
Merged
@clueclue mentioned this pull request Aug 26, 2022
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@clueclueclue approved these changes
@SimonFringsSimonFringsSimonFrings approved these changes
+2 more reviewers
@skydiabloskydiabloskydiablo left review comments
@iorsaiorsaiorsa left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

BC breakmaintenancenew feature

Projects

None yet

Milestone

v0.2.0

Development

Successfully merging this pull request may close these issues.

5 participants

@WyriHaximus@clue@skydiablo@iorsa@SimonFrings