search

BurntSushi / walkdir

Rust library for walking directories recursively.
The Unlicense
1.27k stars 109 forks source link

Option to stay on same filesystem #107

Closed agriffis closed 6 years ago

agriffis commented 6 years ago

This is based on #89 to fix #8, plus changes from Andrew's code review.

agriffis commented 6 years ago

Build failed because of a timeout in Travis. I just re-pushed to see if it will run through

mckaymatt commented 6 years ago

I'd like to add a windows test, but I don't know how and I haven't been able to find the time to figure it out.

agriffis commented 6 years ago

@mckaymatt I don't know either. Windows doesn't have a well-known mounted dir like Linux does. The point of testing in this case is to ensure it works right with the various Windows API calls, so mocking windows_file_handle_info doesn't really make sense.

@BurntSushi Would you consider merging without a Windows test?

agriffis commented 6 years ago

@BurntSushi ping?

BurntSushi commented 6 years ago

Apologies for being a bad reviewer here and taking so long, but I think part of the reason why it took me so long is because I had to play around with this myself before getting to merge it. Once I started playing with it, I noticed several issues, and I don't think I have the energy to take this through another round of feedback. In light of that, I made the requisite changes myself and submitted #110.

Also, for future reference, I would expect a change like this to be a single commit. I had to basically re-capitulate your diff from scratch because it could not be feasibly squashed/rebased.

In any case, thanks so much @agriffis and @mckaymatt for working on this! Very much appreciate the work done and the push to get this merged!

agriffis commented 6 years ago

@BurntSushi Thanks!