rasta-rocket commented on PR #1319: URL: https://github.com/apache/iceberg-go/pull/1319#issuecomment-4826452023
Thanks @zeroshade for the review! I addressed the doc-comment nit and refactored to a single `yaml.Unmarshal` by folding `ParseDefaultCatalog` into `ParseConfig`: an empty `catalogName` now resolves via the file's `default-catalog` field, then falls back to the built-in `"default"`. For the `args_test.go` coverage you mentioned: the precedence logic (explicit flag wins, else pass "" to let ParseConfig resolve from the file) currently lives inline in `main()` and isn't reachable from a test. To cover it, I'd need to extract those few lines into a named helper. Happy to do that, do you have a name in mind? I was thinking something like `catalogConfigFromFile` or `resolveCatalogConfig`, but wanted your preference before adding a new exported/unexported symbol. -- 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]
