flyrain commented on code in PR #3340: URL: https://github.com/apache/polaris/pull/3340#discussion_r2677114592
########## runtime/server/src/main/java/org/apache/polaris/admintool/BootstrapCommand.java: ########## Review Comment: I see the appeal of this approach, but I worry it may dismiss some of the key benefits of the current refactor, and make the dependency structure less clearer. Please note that, the CLI will need `ServiceProvider` to work now. It also makes testing a bit awkward. Tests that are specific to the CLI behavior would have to depend on the `default` and `service` module just to pick up `application-cli` and `application` properties , which feels indirect and less explicit. For those reasons, I think the current direction still has advantages in terms of modularity, clarity, and test ergonomics. -- 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]
