chadlwilson commented on pull request #319: URL: https://github.com/apache/commons-io/pull/319#issuecomment-1023281649
> > If the intent is to re-use NIO logic, I suppose the `Files.copy` could be preceded by `Files.createDirectories(destination.getParentFile().toPath());` > > The usage of `Files.copy` probably also needs `CopyOptions.REPLACE_EXISTING`. Haven't thought through all the other cases either, as there are some missing exception cases here in the tests. > > Hi @chadlwilson Yes, the idea is to use NIO. Would you adjust the PR? I don't really understand the original "NIO" change, because - there doesn't seem to be a linked JIRA issue explaining what is trying to be fixed/addressed? - it wasn't consistent as it was - `FileUtils.copyURLToFile(url, file, connTimeout, readTimeout)` wasn't using the NIO mechanism, so it made the two overloads have different semantics. - the comment was `Use NIO internally to avoid using finalizable FileInputStream.` but I don't understand what this meant. In both cases the input stream was opened on the URL with `url.openStream()` in the calling method, and I don't understand what `FileInputStream` has to do with the change, as the type of stream depends on the URL type? From my perspective, right now `master` is broken and probably not releasable - and I felt it would be better to take it back to a working/releasable state and then re-introduce NIO changes when the intent is clearer and there is sufficient time to ensure there aren't regressions. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
