rdblue commented on a change in pull request #2203:
URL: https://github.com/apache/iceberg/pull/2203#discussion_r569864524
##########
File path:
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalogs.java
##########
@@ -34,6 +36,7 @@ private HiveCatalogs() {
public static HiveCatalog loadCatalog(Configuration conf) {
// metastore URI can be null in local mode
String metastoreUri = conf.get(HiveConf.ConfVars.METASTOREURIS.varname,
"");
- return CATALOG_CACHE.get(metastoreUri, uri -> new HiveCatalog(conf));
+ return CATALOG_CACHE.get(metastoreUri, uri -> (HiveCatalog)
CatalogUtil.loadCatalog(HiveCatalog.class.getName(),
+ "hive", ImmutableMap.of(), conf));
Review comment:
Minor: I find this line wrapping very awkward because at a glance and
from indentation, it looks like these args are passed to `CATALOG_CACHE.get`.
But they are actually inside the lambda and passed to `loadCatalog`.
When breaking lines, I prefer to try not to just break wherever a line gets
too long and instead break at reasonable points for readability. If possible, I
think this should break at `CatalogUtil.loadCatalog` so that the entire method
call is on one line. Otherwise, putting all args on one line makes the most
sense so that at least the previous line ends with `loadCatalog(` and it is
clear that the next line starts the argument list.
Same with the other changes above. Those aren't as bad as this, but I'd
still wrap a little differently (not after the first method arg) to increase
readability.
----------------------------------------------------------------
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]
