----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55999/#review163576 -----------------------------------------------------------
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java (line 28) <https://reviews.apache.org/r/55999/#comment235040> can this be private final? sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java (line 30) <https://reviews.apache.org/r/55999/#comment235041> can this be private final? sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java (line 33) <https://reviews.apache.org/r/55999/#comment235043> These are pretty long lines - can you vreak them? sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterTableMessage.java (line 26) <https://reviews.apache.org/r/55999/#comment235030> Can this be private and final? sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterTableMessage.java (line 28) <https://reviews.apache.org/r/55999/#comment235031> Can this be private and final? sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterTableMessage.java (line 30) <https://reviews.apache.org/r/55999/#comment235044> Please break these long lines sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java (line 29) <https://reviews.apache.org/r/55999/#comment235034> Can this be private and final? sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java (line 35) <https://reviews.apache.org/r/55999/#comment235045> Please break long lines sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java (line 26) <https://reviews.apache.org/r/55999/#comment235052> can it be private? sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java (line 32) <https://reviews.apache.org/r/55999/#comment235053> IMO this can be removed sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java (line 42) <https://reviews.apache.org/r/55999/#comment235054> readValue throws some specific exceptions - can we be more specific here as well? sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java (line 43) <https://reviews.apache.org/r/55999/#comment235055> It isn't that we can't construct it, we can not deserialize it. Same goes for methods below. sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java (line 84) <https://reviews.apache.org/r/55999/#comment235056> This can be converted to foreach loop sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java (line 134) <https://reviews.apache.org/r/55999/#comment235057> This can be converted to foreach loop - Alexander Kolbasov On Jan. 26, 2017, 10:49 p.m., Nachiket Vaidya wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55999/ > ----------------------------------------------------------- > > (Updated Jan. 26, 2017, 10:49 p.m.) > > > Review request for sentry, Alexander Kolbasov and Hao Hao. > > > Bugs: SENTRY-1602 > https://issues.apache.org/jira/browse/SENTRY-1602 > > > Repository: sentry > > > Description > ------- > > SENTRY-1602 Code cleanup for Sentry JSON message factory for hive > notifications > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java > 890186ba7eecd4aace280d9a1b56a8f01b625b77 > > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java > 76211c35f4f5ed158b5c3052f7288cd0083f0d52 > > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java > 99eb67a61363616af663a9be579b2e3a3344fd69 > > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterTableMessage.java > 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62 > > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java > 2ab61f7ad5d30a1edb454c3cb239532c736b52e6 > > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java > b645c4504ce6e768b31e6f7a1a39b60f73e1ac64 > > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java > 00e7db8ea4ccf92dae58869fcb21e6e2fcb27103 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java > 567b4c86339a9494647dccc980df2b427225907f > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestSentryListenerSentryDeserializer.java > 6f1886f942ca80ab4c356b81777d3ac336017292 > > Diff: https://reviews.apache.org/r/55999/diff/ > > > Testing > ------- > > Ran unit test cases and they passed. > > > Thanks, > > Nachiket Vaidya > >