Jim Apple has posted comments on this change. Change subject: REVIEW-ONLY: the results of running clang-format ......................................................................
Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/4046/1/be/src/exec/hash-table.cc File be/src/exec/hash-table.cc: PS1, Line 896: if (build_expr_ctxs_[i]->root()->type().type != TYPE_STRING && : build_expr_ctxs_[i]->root()->type().type != TYPE_VARCHAR) : continue; > This is an issue: we don't want then-clauses on a different line from the i I don't see a way to get clang-format to fix that. http://gerrit.cloudera.org:8080/#/c/4046/1/be/src/exec/parquet-common.h File be/src/exec/parquet-common.h: PS1, Line 40: // Invalid : parquet::Type::BOOLEAN, // NULL type : parquet::Type::BOOLEAN, parquet::Type::INT32, parquet::Type::INT32, : parquet::Type::INT32, parquet::Type::INT64, parquet::Type::FLOAT, : parquet::Type::DOUBLE, : parquet::Type::INT96, // Timestamp : parquet::Type::BYTE_ARRAY, // String : parquet::Type::BYTE_ARRAY, // Date, NYI : parquet::Type::BYTE_ARRAY, // DateTime, NYI : parquet::Type::BYTE_ARRAY, // Binary NYI : parquet::Type::FIXED_LEN_BYTE_ARRAY, // Decimal : parquet::Type::BYTE_ARRAY, // VARCHAR(N) : parquet::Type::BYTE_ARRAY, // CHAR(N) > does this seem weird to anyone else - the mix of packed types and one-per-l Yeah, this is not great. I don't see a way to fix this in clang-format's config, unfortunately. http://gerrit.cloudera.org:8080/#/c/4046/1/be/src/util/metrics.h File be/src/util/metrics.h: Line 63: MetricDefs(){}; > what's with the missing space here? Not sure. I don't see any config parameters to tune this. PS1, Line 221: /// Typically a metric object is cached by its creator after registration. If a metric : /// must : /// be retrieved without an available pointe > not sure why it doesn't properly combine lines 222 and 223 I've noticed this as well. I think the reasoning is that the user put a line break in the comment, maybe they really meant it. -- To view, visit http://gerrit.cloudera.org:8080/4046 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If00bcdb7957ec55a3b8568887de161b7f30d58de Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-HasComments: Yes
