[Impala-ASF-CR] Match .clang-format more closely to actual practice.
Jim Apple has submitted this change and it was merged. Change subject: Match .clang-format more closely to actual practice. .. Match .clang-format more closely to actual practice. In order to attempt to get code like double VeryLongFunctionNames(double x1, double x2, double x3, double x4) { return 1.0; } rather than double VeryLongFunctionNames( double x1, double x2, double x3, double x4) { return 1.0; } I wrote a small set of programs to infer which .clang-format params fit the current Impala codebase most closely; this patch is the result. This patch is the best the inferencer found (while maintaining certain enforced parameters, like 90-character lines). It is about 10% closer to Impala's current code base than the .clang-format that is checked in at the moment, as measured by number of lines in the diff. Change-Id: Iccaec6c1673c3e08d2c39200b0c84437af629aed Reviewed-on: http://gerrit.cloudera.org:8080/4590 Reviewed-by: Jim AppleTested-by: Jim Apple --- M .clang-format 1 file changed, 35 insertions(+), 14 deletions(-) Approvals: Jim Apple: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/4590 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iccaec6c1673c3e08d2c39200b0c84437af629aed Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Match .clang-format more closely to actual practice.
Jim Apple has posted comments on this change. Change subject: Match .clang-format more closely to actual practice. .. Patch Set 3: Verified+1 Can't break tests with this change, so +1 verify manually -- To view, visit http://gerrit.cloudera.org:8080/4590 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccaec6c1673c3e08d2c39200b0c84437af629aed Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim AppleGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] Match .clang-format more closely to actual practice.
Jim Apple has posted comments on this change. Change subject: Match .clang-format more closely to actual practice. .. Patch Set 3: Code-Review+2 rebase, carry +1 into +2 -- To view, visit http://gerrit.cloudera.org:8080/4590 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccaec6c1673c3e08d2c39200b0c84437af629aed Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim AppleGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] Match .clang-format more closely to actual practice.
Jim Apple has posted comments on this change. Change subject: Match .clang-format more closely to actual practice. .. Patch Set 2: > > I'm not sure I understand why a person might want to run the > inferencer again, even if the relationship between config files and > the induced changes are sometimes confusing. > > If I don't understand how to get the results I want (because the > formatter has become a black box), the only recourse left to me is > to ask a machine to do it for me - just like has happened here. I think I understand what you mean. Unfortunately, I think that's an issue with the number of clang-format options. I think we will face that issue with or without inference. -- To view, visit http://gerrit.cloudera.org:8080/4590 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccaec6c1673c3e08d2c39200b0c84437af629aed Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim AppleGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] Match .clang-format more closely to actual practice.
Henry Robinson has posted comments on this change. Change subject: Match .clang-format more closely to actual practice. .. Patch Set 2: Do you understand the relationship between the resulting .clang-format file and the changes in formatting? I would rather not get into a situation where I have to re-run an inferencer to make small changes to the format file because we don't understand the dependency of the outputs on the inputs. FWIW I'm quite happy with the current formatter, but willing to cede to others if they feel strongly. -- To view, visit http://gerrit.cloudera.org:8080/4590 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccaec6c1673c3e08d2c39200b0c84437af629aed Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim AppleGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] Match .clang-format more closely to actual practice.
Tim Armstrong has posted comments on this change. Change subject: Match .clang-format more closely to actual practice. .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/4590 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccaec6c1673c3e08d2c39200b0c84437af629aed Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim AppleGerrit-Reviewer: Jim Apple Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] Match .clang-format more closely to actual practice.
Jim Apple has uploaded a new patch set (#2). Change subject: Match .clang-format more closely to actual practice. .. Match .clang-format more closely to actual practice. In order to attempt to get code like double VeryLongFunctionNames(double x1, double x2, double x3, double x4) { return 1.0; } rather than double VeryLongFunctionNames( double x1, double x2, double x3, double x4) { return 1.0; } I wrote a small set of programs to infer which .clang-format params fit the current Impala codebase most closely; this patch is the result. This patch is the best the inferencer found (while maintaining certain enforced parameters, like 90-character lines). It is about 10% closer to Impala's current code base than the .clang-format that is checked in at the moment, as measured by number of lines in the diff. Change-Id: Iccaec6c1673c3e08d2c39200b0c84437af629aed --- M .clang-format 1 file changed, 35 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/4590/2 -- To view, visit http://gerrit.cloudera.org:8080/4590 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iccaec6c1673c3e08d2c39200b0c84437af629aed Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim AppleGerrit-Reviewer: Jim Apple Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Match .clang-format more closely to actual practice.
Tim Armstrong has posted comments on this change. Change subject: Match .clang-format more closely to actual practice. .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/4590/1/.clang-format File .clang-format: Line 6 Putting short if statements on a single line is something we've definitely encouraged. Does this make the diff smaller? Line 34: DerivePointerAlignment: false This doesn't seem relevant for Java. -- To view, visit http://gerrit.cloudera.org:8080/4590 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccaec6c1673c3e08d2c39200b0c84437af629aed Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim AppleGerrit-Reviewer: Jim Apple Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes