-----------------------------------------------------------
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
> 
>

Reply via email to