msfroh commented on PR #16173: URL: https://github.com/apache/lucene/pull/16173#issuecomment-5197826832
Yeah -- I really struggle to see an argument for not delegating `copyFrom`. In particular, if someone does want to override it with a wrapper or a `Directory` subclass, it likely means that they have a specific implementation that helps improve copying (like `HardlinkCopyDirectoryWrapper` or a `Directory` implementation backed by an object store, which can implement the copy as a server-side operation). Looking at the other non-test overrides of the method, I think they're both incorrect. Both `LockValidatingDirectoryWrapper` and `TrackingDirectoryWrapper` implement `copyFrom`, but their implementations don't do much beyond what `createOutput` would already. (I guess `LockValidatingDirectoryWrapper` checks that we have the lock before calling `openInput` and then checks again in the `createOutput` call.) I'm going to suggest that we should merge this. It's got my approval. In case anyone else has a good counterargument, I'll merge in a couple of days. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
