You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Andrew Sherman (Code Review)" <ge...@cloudera.org> on 2019/01/05 01:29:41 UTC
[Impala-ASF-CR] IMPALA-8047 Support .proto files in .clang-format
Andrew Sherman has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12165
Change subject: IMPALA-8047 Support .proto files in .clang-format
......................................................................
IMPALA-8047 Support .proto files in .clang-format
The .proto file extension is used for the Google Protocol Buffers
language. Impala uses this language to specify the format of messages
used by KRPC. Add support for this language to .clang-format so that we
can have consistent formatting.
The proposed support is:
Language: Proto
BasedOnStyle: Google
ColumnLimit: 90
This produces only a few diffs when run against the existing Impala
code. I’m not proposing to make any changes to .proto files, this is
just to show what clang-format will do. Apart from wrapping comments and
code at 90 chars, the diffs are mostly of the form
-syntax="proto2";
+syntax = "proto2";
- message Certificate {};
+ message Certificate {
+ };
- optional bool client_timeout_defined = 4 [ default = false ];
+ optional bool client_timeout_defined = 4 [default = false];
- UNKNOWN = 999;
- NEGOTIATE = 1;
- SASL_SUCCESS = 0;
- SASL_INITIATE = 2;
+ UNKNOWN = 999;
+ NEGOTIATE = 1;
+ SASL_SUCCESS = 0;
+ SASL_INITIATE = 2;
This last change can be configured using “AlignConsecutiveAssignments:
true” but that creates a different set of diffs.
Change-Id: I0c2dcfc21fc8f9206adb64166fbd05580516791f
---
M .clang-format
1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/12165/2
--
To view, visit http://gerrit.cloudera.org:8080/12165
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0c2dcfc21fc8f9206adb64166fbd05580516791f
Gerrit-Change-Number: 12165
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
[Impala-ASF-CR] IMPALA-8047 Support .proto files in .clang-format
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12165 )
Change subject: IMPALA-8047 Support .proto files in .clang-format
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1719/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/12165
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c2dcfc21fc8f9206adb64166fbd05580516791f
Gerrit-Change-Number: 12165
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sat, 05 Jan 2019 02:01:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8047 Support .proto files in .clang-format
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12165 )
Change subject: IMPALA-8047 Support .proto files in .clang-format
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12165/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12165/2//COMMIT_MSG@20
PS2, Line 20: This produces only a few diffs when run against the existing Impala
Thanks for including this, made it way easier to review.
--
To view, visit http://gerrit.cloudera.org:8080/12165
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c2dcfc21fc8f9206adb64166fbd05580516791f
Gerrit-Change-Number: 12165
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 07 Jan 2019 17:57:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8047 Support .proto files in .clang-format
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12165 )
Change subject: IMPALA-8047 Support .proto files in .clang-format
......................................................................
IMPALA-8047 Support .proto files in .clang-format
The .proto file extension is used for the Google Protocol Buffers
language. Impala uses this language to specify the format of messages
used by KRPC. Add support for this language to .clang-format so that we
can have consistent formatting.
The proposed support is:
Language: Proto
BasedOnStyle: Google
ColumnLimit: 90
This produces only a few diffs when run against the existing Impala
code. I’m not proposing to make any changes to .proto files, this is
just to show what clang-format will do. Apart from wrapping comments and
code at 90 chars, the diffs are mostly of the form
-syntax="proto2";
+syntax = "proto2";
- message Certificate {};
+ message Certificate {
+ };
- optional bool client_timeout_defined = 4 [ default = false ];
+ optional bool client_timeout_defined = 4 [default = false];
- UNKNOWN = 999;
- NEGOTIATE = 1;
- SASL_SUCCESS = 0;
- SASL_INITIATE = 2;
+ UNKNOWN = 999;
+ NEGOTIATE = 1;
+ SASL_SUCCESS = 0;
+ SASL_INITIATE = 2;
This last change can be configured using “AlignConsecutiveAssignments:
true” but that creates a different set of diffs.
Change-Id: I0c2dcfc21fc8f9206adb64166fbd05580516791f
Reviewed-on: http://gerrit.cloudera.org:8080/12165
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M .clang-format
1 file changed, 4 insertions(+), 0 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/12165
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0c2dcfc21fc8f9206adb64166fbd05580516791f
Gerrit-Change-Number: 12165
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-8047 Support .proto files in .clang-format
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12165 )
Change subject: IMPALA-8047 Support .proto files in .clang-format
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12165
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c2dcfc21fc8f9206adb64166fbd05580516791f
Gerrit-Change-Number: 12165
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 07 Jan 2019 17:57:44 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8047 Support .proto files in .clang-format
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12165 )
Change subject: IMPALA-8047 Support .proto files in .clang-format
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3615/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12165
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c2dcfc21fc8f9206adb64166fbd05580516791f
Gerrit-Change-Number: 12165
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 07 Jan 2019 17:57:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8047 Support .proto files in .clang-format
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12165 )
Change subject: IMPALA-8047 Support .proto files in .clang-format
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12165
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c2dcfc21fc8f9206adb64166fbd05580516791f
Gerrit-Change-Number: 12165
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 07 Jan 2019 21:55:22 +0000
Gerrit-HasComments: No