----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68889/#review209131 -----------------------------------------------------------
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java Lines 123 (patched) <https://reviews.apache.org/r/68889/#comment293421> General comment: don't try to do too much in static initializers in server code. Just like in HIVE-20545 you have to consider what will happen if there is a failure during initialization, and the result is always ugly. In this case it looks safe but IT MADE ME THINK which is bad. itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java Line 802 (original), 804 (patched) <https://reviews.apache.org/r/68889/#comment293422> This looks ugly to me. I think the string concatenation operator + should be separated on both sides by spaces. I think that is what is most commonly used on Hive - I'll leave it to you to check. But the usage is here is different from that in the static initializer code and that inconsistency is ugly too. IMHO You should teach Intellij to do your formatting and then let it decide this stuff - Andrew Sherman On Oct. 1, 2018, 7:09 a.m., Bharathkrishna Guruvayoor Murali wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68889/ > ----------------------------------------------------------- > > (Updated Oct. 1, 2018, 7:09 a.m.) > > > Review request for hive, Alexander Kolbasov and Andrew Sherman. > > > Bugs: HIVE-20610 > https://issues.apache.org/jira/browse/HIVE-20610 > > > Repository: hive-git > > > Description > ------- > > Adding java.io.tmpdir as tmp directory instead of /tmp > > > Diffs > ----- > > > itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java > 82429e36a5 > > > Diff: https://reviews.apache.org/r/68889/diff/1/ > > > Testing > ------- > > > Thanks, > > Bharathkrishna Guruvayoor Murali > >
