nastra commented on a change in pull request #2826:
URL: https://github.com/apache/iceberg/pull/2826#discussion_r690999266
##########
File path: core/src/test/java/org/apache/iceberg/TestTableMetadata.java
##########
@@ -90,11 +90,11 @@ public void testJsonConversion() throws Exception {
long previousSnapshotId = System.currentTimeMillis() - new
Random(1234).nextInt(3600);
Snapshot previousSnapshot = new BaseSnapshot(
ops.io(), previousSnapshotId, null, previousSnapshotId, null, null,
null, ImmutableList.of(
- new GenericManifestFile(localInput("file:/tmp/manfiest.1.avro"),
SPEC_5.specId())));
+ new GenericManifestFile(localInput("file:/tmp/manfiest.1.avro"),
SPEC_5.specId())));
Review comment:
yes these are required
##########
File path:
hive3/src/main/java/org/apache/iceberg/mr/hive/vector/CompatibilityHiveVectorUtils.java
##########
@@ -59,7 +59,7 @@ private CompatibilityHiveVectorUtils() {
* Returns serialized mapwork instance from a job conf - ported from Hive
source code LlapHiveUtils#findMapWork
*
* @param job JobConf instance
- * @return
+ * @return A serialized {@link MapWork} based on the given job conf
Review comment:
I searched across the codebase and it seems that it's a bit inconsistent
in terms of upper/lowercase. I generally think that something like this would
be better if addressed by Checkstyle (but not sure if checkstyle supports such
a check). Otherwise stuff like this will essentially pop up in lots of PR
reviews
##########
File path: mr/src/test/java/org/apache/iceberg/mr/hive/TestTables.java
##########
@@ -115,7 +115,7 @@ public String catalogName() {
/**
* The table properties string needed for the CREATE TABLE ... commands,
* like "TBLPROPERTIES('iceberg.catalog'='mycatalog')
- * @return
+ * @return The tables properties string, such as
"TBLPROPERTIES('iceberg.catalog'='mycatalog')
Review comment:
ah this is just a blind copy/paste from the javadoc description a few
lines above without actually me checking whether it had missing pieces..will
update
--
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]