[Impala-ASF-CR] IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp
Kim Jin Chul 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 5: Jim, thanks for your review! -- 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: 5 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Tue, 19 Dec 2017 00:03:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8829 ) Change subject: IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp .. IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp In the commit 4feb4f3a, the third party library pcg-cpp was excluded from the clang-tidy check. It could make unexpected side effect, so fixing some warnings from clang-tidy is better than avoidance of the check. Change-Id: I591d30373cb13f0eb89afbe16d81b1d3fb783365 Reviewed-on: http://gerrit.cloudera.org:8080/8829 Reviewed-by: Jim Apple Tested-by: Impala Public Jenkins --- M be/src/thirdparty/pcg-cpp-0.98/include/pcg_extras.hpp M bin/run_clang_tidy.sh 2 files changed, 13 insertions(+), 2 deletions(-) Approvals: Jim Apple: Looks good to me, approved Impala Public Jenkins: Verified -- 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: merged Gerrit-Change-Id: I591d30373cb13f0eb89afbe16d81b1d3fb783365 Gerrit-Change-Number: 8829 Gerrit-PatchSet: 5 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul
[Impala-ASF-CR] IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp
Impala Public Jenkins 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 4: Verified+1 -- 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: 4 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Mon, 18 Dec 2017 18:37:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp
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 4: Code-Review+2 -- 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: 4 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Mon, 18 Dec 2017 14:57:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp
Impala Public Jenkins 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 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1635/ -- 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: 4 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Mon, 18 Dec 2017 14:57:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp
Kim Jin Chul 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) 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 Done 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. Done 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 > nit: can you reword slightly - it's not just that clang-tidy complains, it' I agree. It is better to show a main reason why disable the code. -- 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 Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Mon, 18 Dec 2017 05:14:00 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp
Hello Jim Apple, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8829 to look at the new patch set (#3). Change subject: IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp .. IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp In the commit 4feb4f3a, the third party library pcg-cpp was excluded from the clang-tidy check. It could make unexpected side effect, so fixing some warnings from clang-tidy is better than avoidance of the check. Change-Id: I591d30373cb13f0eb89afbe16d81b1d3fb783365 --- M be/src/thirdparty/pcg-cpp-0.98/include/pcg_extras.hpp M bin/run_clang_tidy.sh 2 files changed, 13 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/8829/3 -- 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: newpatchset Gerrit-Change-Id: I591d30373cb13f0eb89afbe16d81b1d3fb783365 Gerrit-Change-Number: 8829 Gerrit-PatchSet: 3 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul
[Impala-ASF-CR] IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp
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 Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Mon, 18 Dec 2017 04:19:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp
Kim Jin Chul 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: (1 comment) 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@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 thought comment out is better than removal of the code because someone needs this later. -- 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 Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Wed, 13 Dec 2017 13:38:35 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp
Hello Jim Apple, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8829 to look at the new patch set (#2). Change subject: IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp .. IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp In the commit 4feb4f3a, the third party library pcg-cpp was excluded from the clang-tidy check. It could make unexpected side effect, so fixing some warnings from clang-tidy is better than avoidance of the check. Change-Id: I591d30373cb13f0eb89afbe16d81b1d3fb783365 --- M be/src/thirdparty/pcg-cpp-0.98/include/pcg_extras.hpp M bin/run_clang_tidy.sh 2 files changed, 8 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/8829/2 -- 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: newpatchset Gerrit-Change-Id: I591d30373cb13f0eb89afbe16d81b1d3fb783365 Gerrit-Change-Number: 8829 Gerrit-PatchSet: 2 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple
[Impala-ASF-CR] IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp
Kim Jin Chul has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8829 Change subject: IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp .. IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp In the commit 4feb4f3a, the third party library pcg-cpp was excluded from the clang-tidy check. It could make unexpected side effect, so fixing some warnings from clang-tidy is better than avoidance of the check. Change-Id: I591d30373cb13f0eb89afbe16d81b1d3fb783365 --- M be/src/thirdparty/pcg-cpp-0.98/include/pcg_extras.hpp M bin/run_clang_tidy.sh 2 files changed, 4 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/8829/1 -- 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: newchange Gerrit-Change-Id: I591d30373cb13f0eb89afbe16d81b1d3fb783365 Gerrit-Change-Number: 8829 Gerrit-PatchSet: 1 Gerrit-Owner: Kim Jin Chul