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

Added Windows remove_dir_all implementation from #31944 #17

Merged
merged 1 commit into from May 4, 2017

Conversation

Projects
None yet
5 participants
@Aaronepower
Copy link
Contributor

Aaronepower commented Nov 5, 2016

fixes #15

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Nov 8, 2016

Thanks for the PR! Perhaps this could use winapi instead of vendoring large parts of the standard library? (may also help readability as well)

@retep998

This comment has been minimized.

Copy link

retep998 commented Nov 9, 2016

Practically all of that windows api stuff should be available in winapi.

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Nov 17, 2016

Neat! I agree that it would be better to use winapi here, and I'd also prefer if you would publish remove_dir_all as a crate so others can use it as well.

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Mar 11, 2017

Next thing to do here is extract the function to its own crate, convert it to use winapi and then use that crate in tempdir.

@Aaronepower

This comment has been minimized.

Copy link
Contributor Author

Aaronepower commented Mar 29, 2017

I have pulled it out into a new crate, remove_dir_all which now uses winapi.

@pwoolcoc

This comment has been minimized.

Copy link
Contributor

pwoolcoc commented Mar 31, 2017

@Aaronepower I opened a pr on remove_dir_all that should fix the build issue

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Apr 7, 2017

@Aaronepower thank you! Looks like there's a bit more to do here before merging, but thanks for picking it up again.

@Aaronepower

This comment has been minimized.

Copy link
Contributor Author

Aaronepower commented Apr 25, 2017

@brson I've updated it with @pwoolcoc 's changes. Seems like it works fine now!

@brson brson merged commit 8c3ed02 into rust-lang-deprecated:master May 4, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@brson

This comment has been minimized.

Copy link
Contributor

brson commented May 4, 2017

Thanks @Aaronepower!

opilar pushed a commit to opilar/tempdir that referenced this pull request Sep 22, 2017

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