Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/9584 )
Change subject: IMPALA-5980: Upgrade to LLVM 5.0.1 ...................................................................... Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/9584/4/be/src/rpc/impala-service-pool.cc File be/src/rpc/impala-service-pool.cc: http://gerrit.cloudera.org:8080/#/c/9584/4/be/src/rpc/impala-service-pool.cc@80 PS4, Line 80: ImpalaServicePool::Shutdown(); > This was required because Shutdown() is virtual. However, I don't see why S I agree, will remove the virtual keyword from both. http://gerrit.cloudera.org:8080/#/c/9584/4/be/src/transport/TSasl.cpp File be/src/transport/TSasl.cpp: http://gerrit.cloudera.org:8080/#/c/9584/4/be/src/transport/TSasl.cpp@57 PS4, Line 57: (const char*) > Switch this cast too? Done http://gerrit.cloudera.org:8080/#/c/9584/4/be/src/transport/TSasl.cpp@72 PS4, Line 72: (const char* > Switch this cast too? Done http://gerrit.cloudera.org:8080/#/c/9584/4/be/src/transport/TSasl.cpp@242 PS4, Line 242: (const char*) > Switch this cast and the one just below too? Done http://gerrit.cloudera.org:8080/#/c/9584/4/be/src/udf/udf-test.cc File be/src/udf/udf-test.cc: http://gerrit.cloudera.org:8080/#/c/9584/4/be/src/udf/udf-test.cc@140 PS4, Line 140: // ptime t(*(date*)&time.date); is this conversion correct? > Leftover comment. This looks right to me. oops forgot to remove the comment. -- To view, visit http://gerrit.cloudera.org:8080/9584 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib0a15cb53feab89e7b35a56b67b3b30eb3e62c6b Gerrit-Change-Number: 9584 Gerrit-PatchSet: 4 Gerrit-Owner: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Tue, 27 Mar 2018 23:55:05 +0000 Gerrit-HasComments: Yes
