aplex commented on a change in pull request #3158:
URL: https://github.com/apache/incubator-gobblin/pull/3158#discussion_r530649551
##########
File path:
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/hive/HiveTargetDirectoryClient.java
##########
@@ -0,0 +1,16 @@
+package org.apache.gobblin.data.management.copy.hive;
+
+import java.io.IOException;
+import java.util.Properties;
+import org.apache.hadoop.fs.Path;
+
+
+public class HiveTargetDirectoryClient {
Review comment:
You can add some docs all around, explaining how this can be used and
extended
##########
File path:
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/hive/HiveCopyEntityHelper.java
##########
@@ -351,11 +358,21 @@
} else {
this.existingTargetTable = Optional.absent();
}
-
Review comment:
This constructor is too complex, and it's hard to follow what's going on
here. You can look if it can be refactored and split into smaller functions. If
the refactoring is big and touches a lot of code, you can send it as a separate
PR, before you main change.
You can also install this IntelliJ plugin that will tell you if the function
is too complex, right in the IDE:
https://plugins.jetbrains.com/plugin/12159-codemetrics
----------------------------------------------------------------
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]
