Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16260 )
Change subject: Add .clang-format file ...................................................................... Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/16260/1/docs/contributing.adoc File docs/contributing.adoc: http://gerrit.cloudera.org:8080/#/c/16260/1/docs/contributing.adoc@205 PS1, Line 205: git show -U0 | thirdparty/installed/uninstrumented/share/clang/clang-format-diff.py -i -p1 If clang-format isn't available in the PATH, this output an error. I think that specifying '-binary' option for the clang-format-diff.py would be more consistent in that regard: git show -U0 | thirdparty/installed/uninstrumented/share/clang/clang-format-diff.py -binary thirdparty/installed/uninstrumented/bin/clang-format -p1 -i http://gerrit.cloudera.org:8080/#/c/16260/1/src/kudu/.clang-format File src/kudu/.clang-format: http://gerrit.cloudera.org:8080/#/c/16260/1/src/kudu/.clang-format@18 PS1, Line 18: BasedOnStyle: Google : SortIncludes: false : DerivePointerAlignment: false : ColumnLimit: 98 : BinPackArguments: false : BinPackParameters: false nit: it would be nice to add comments per-directive why it's there, if possible. For example, I guess 'SortIncludes' is set to 'false' because it would conflict with IWYU sorting, right? -- To view, visit http://gerrit.cloudera.org:8080/16260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I41def9a77bd98bf09353fe9a7789756a1bff30c1 Gerrit-Change-Number: 16260 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 30 Jul 2020 23:40:29 +0000 Gerrit-HasComments: Yes
