-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
fix: for #6374 to delete non-empty directories #6375
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thank you for reporting and fixing the issue. Really appreciate this. |
@@ -91,7 +91,10 @@ public static boolean delete(@Nullable File file) { | |||
} | |||
|
|||
try { | |||
Files.delete(file.toPath()); | |||
Files.walk(file.toPath()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The stream returned by Files.walk should be closed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting... most streams don't need to be closed like this; as documented in the Stream javadoc. Hadn't thought about this possibly being tied to an IO channel. Turns out this isn't uncommon: https://sourcegraph.com/search?q=context:global+content:%22try+%28Stream%3CPath%3E+paths+%3D+Files.walk%28%22+lang:java&patternType=standard&sm=1&groupBy=repo
Fixed with #6382
Thanks!
Fixes Issue #6374
Description of Change
Walk the files to delete using just java.
Have test cases been added to cover the new functionality?
yes