Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12174 )
Change subject: Assign locations to tablet servers and the client in Java ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java File java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java: http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java@49 PS3, Line 49: -- nit: ' --' or replace with ';' http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java: PS3: > What happened to the formatting in this file? I guess the very first version of this file was had DOS end-of-line symbols, and it's still partially true. Will, while you are at at, maybe run dos2unix for this file? http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java@1 PS3, Line 1: /** : * Licensed under the Apache License, Version 2.0 (the "License"); : * you may not use this file except in compliance with the License. : * You may obtain a copy of the License at : * : * http://www.apache.org/licenses/LICENSE-2.0 : * : * Unless required by applicable law or agreed to in writing, software : * distributed under the License is distributed on an "AS IS" BASIS, : * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. : * See the License for the specific language governing permissions and : * limitations under the License. See accompanying LICENSE file. : */ nit: while you are at it, maybe unify the license header and make it commented by single-line comments? Also, it seems the text is little bit different from the standard license header, but I'm not sure whether it's crucial to standardize that as well. http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java File java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java: http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java@229 PS3, Line 229: getClass().getResource("/assign-location.py").getFile() I'm curious whether this works when tests are run using dist-test. -- To view, visit http://gerrit.cloudera.org:8080/12174 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e2c74ab12f7da187bf6e75d42a3089bc20235db Gerrit-Change-Number: 12174 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Jan 2019 15:22:21 +0000 Gerrit-HasComments: Yes
