-
Notifications
You must be signed in to change notification settings - Fork 669
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
IOUtils chain together IOException for Closeables, add overload #300
base: master
Are you sure you want to change the base?
Conversation
95847a1
to
d97b791
Compare
-1: This makes no sense to me as the exception are not causaly unrelated and you are forcing them to be. This might be a use case for IOExceptionList. |
Hi @garydgregory , good suggestion, IOExceptionList seems like the perfect fit for this. I have revised the code to use this class. |
I ended up reworking the underlying so that now the method looks like this
See also |
Hi @wodencafe Thank you for your update. Now I see one new method, without any tests, and not used from anywhere.
|
Hello @garydgregory , my apologies, I got swamped this weekend and haven't updated this PR with tests. I also agree that the [] should be changed to ... varargs and swapped to be the last parameter, so I will implement this change, and see if there are other places in Commons IO that can take advantage of this new functionality. I will add tests for this today. Thank you for reviewing the changes, and the contributions and feedback. |
03f7fcd
to
51759a5
Compare
51759a5
to
8097e96
Compare
d56a425
to
1ccb075
Compare
Hi @garydgregory , I have updated the pull request with the changes you suggested. |
This pull request chains together IOExceptions thrown from closing multiple Closeable instances, which allows the method to attempt to close all of the Closeable instances rather than halting at the first IOException. An overload is added to allow closing multiple Closeable instances and passing the possible resulting IOException to an IOConsumer.