Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/8829 )
Change subject: IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp ...................................................................... Patch Set 2: (3 comments) Thanks for taking care of this! Nice work, as usual. http://gerrit.cloudera.org:8080/#/c/8829/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8829/2//COMMIT_MSG@11 PS2, Line 11: ck. nit: long line http://gerrit.cloudera.org:8080/#/c/8829/2/be/src/thirdparty/pcg-cpp-0.98/include/pcg_extras.hpp File be/src/thirdparty/pcg-cpp-0.98/include/pcg_extras.hpp: http://gerrit.cloudera.org:8080/#/c/8829/2/be/src/thirdparty/pcg-cpp-0.98/include/pcg_extras.hpp@37 PS2, Line 37: // To avoid of warning: 'PCG_USE_ZEROCHECK_ROTATE_IDIOM' is not defined, evaluates to 0 Please wrap each in an ifndef block. http://gerrit.cloudera.org:8080/#/c/8829/2/be/src/thirdparty/pcg-cpp-0.98/include/pcg_extras.hpp@599 PS2, Line 599: * warning: expansion of date or time macro is not reproducible > The struct is not used in our code and pcg-cpp's sample and test codes. I t nit: can you reword slightly - it's not just that clang-tidy complains, it's that we don't want to introduce non-reproducability into the system, because it makes bugs harder to diagnose. -- To view, visit http://gerrit.cloudera.org:8080/8829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I591d30373cb13f0eb89afbe16d81b1d3fb783365 Gerrit-Change-Number: 8829 Gerrit-PatchSet: 2 Gerrit-Owner: Kim Jin Chul <jinc...@gmail.com> Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org> Gerrit-Reviewer: Kim Jin Chul <jinc...@gmail.com> Gerrit-Comment-Date: Mon, 18 Dec 2017 04:19:20 +0000 Gerrit-HasComments: Yes
