Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fixes #635 use mv instead of rename as that does not support cross volume operations #1214

Merged
merged 6 commits into from
Jan 3, 2017
Merged

Fixes #635 use mv instead of rename as that does not support cross volume operations #1214

merged 6 commits into from
Jan 3, 2017

Conversation

flaviocopes
Copy link
Contributor

@flaviocopes flaviocopes commented Dec 14, 2016 •
edited
Loading

Still TODO:

  • Check exec() is not disabled
  • Fallback to rename()
  • Check on windows

@flaviocopes flaviocopes mentioned this pull request Dec 14, 2016
Closed
@bauer01
Copy link

bauer01 commented Dec 15, 2016 •
edited
Loading

Tested on Windows 10 x64
Docker Toolbox v.1.13.0-rc3
PHP 7 container
Installing AboutMe plugin

  1. PR branch feature/fix-rename - WORKS
  2. switch back to develop - ERROR

So PR seems to be fine on Windows ;-)

@bauer01
Copy link

bauer01 commented Jan 2, 2017 •
edited
Loading

is this ok? when you plan to merge into develop?

@flaviocopes
Copy link
Contributor Author

I've just requested an additional review from our other team devs 👍

rhukster
rhukster approved these changes Jan 2, 2017
View reviewed changes
Copy link
Member

@rhukster rhukster left a comment

Choose a reason for hiding this comment

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

looks good to me.. should be nicely backwards compatible now with existing behavior if mv is not supported.

@rhukster
Copy link
Member

rhukster commented Jan 3, 2017

@flaviocopes pls fix that CHANGELOG conflict and we can get this merged.

@flaviocopes flaviocopes merged commit 335c443 into develop Jan 3, 2017
@flaviocopes
Copy link
Contributor Author

Merged

@flaviocopes flaviocopes deleted the feature/fix-rename branch January 3, 2017 08:28
flaviocopes added a commit that referenced this pull request Jan 3, 2017
@flaviocopes
Add a better fix for #635. Overwrites #1214
6100536
@flaviocopes
Copy link
Contributor Author

Replaced by 6100536

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@w00fz w00fz w00fz approved these changes

@rhukster rhukster rhukster approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@flaviocopes @bauer01 @rhukster @w00fz @smz