elek commented on pull request #1088: URL: https://github.com/apache/hadoop-ozone/pull/1088#issuecomment-659250951
Thanks the updated @smengcl. And sorry for the later answer. I was ooo. > There are multiple changes in this patch: >> I'll revise the jira title to more accurately reflect my changes in a bit. Still, it contains 3 changes (META-INF changes, defaultFs=ofs change, code structure refactor). You can make the review easier (and faster) by separating them to different patches. > I'm in favor of A. I'll attempt to remove the usage of OzoneClientAdapter in OFS altogether then. I am fine with that approach but let me add some comments to the latest patch. The naming of `BasicRootedOzoneFileSystem` and `BasicRootedOzoneFileSystemImpl` is misleading. Usually the `Impl` postfix is used when the class implemented a well known interface. There is no such interface here. (It's more like the delegation design pattern not an implementation) As a test: Can you please explain what are the differences between the two classes and the responsibilities? If not, we don't need two classes. Just remove the `Impl` and remove the dedicated methods and directly call the proxy from the original methods of `BasicRootedOzoneFileSystem`. Wouldn't it be more simple? ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
