Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12949 )
Change subject: KUDU-2689: Made PartialRow setters use a fluent-style. ...................................................................... Patch Set 1: (2 comments) > Changing a void method to return a value would be source compatible but not > binary compatible. I am not sure if we have a policy on breaking binary > compatibility in the client. In the C++ client we care deeply about preserving ABI compatibility. Our contract is: if we break ABI or API compatibility in the public API surface, we must rev the C++ client library's SOVERSION. This hasn't happened yet. In the Java ecosystem I think there's a stronger culture of rebuilding applications from source when revving dependencies. Client JARs specifically are easier to deal with, as they are typically bundled with applications rather than with servers, so if there's going to be a version mismatch it's usually between the client and the server rather than the client and the application. Anyway, I think it's worth investigating whether we've broken ABI compatibility before, as we can use that to inform this decision. > If we are concerned about breaking binary compatibility we should probably > setup a tool to check for breakage between releases. Agreed. Is there something off the shelf we can use here? http://gerrit.cloudera.org:8080/#/c/12949/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java File java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java: http://gerrit.cloudera.org:8080/#/c/12949/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@1346 PS1, Line 1346: return this; There's an extra space here between 'return' and 'this'. http://gerrit.cloudera.org:8080/#/c/12949/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java: http://gerrit.cloudera.org:8080/#/c/12949/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java@471 PS1, Line 471: There are two spaces here. -- To view, visit http://gerrit.cloudera.org:8080/12949 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3476fb3c0a28812e2732e6c6268f31a393928206 Gerrit-Change-Number: 12949 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward <raym...@phdata.io> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Brian McDevitt <br...@phdata.io> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Mon, 08 Apr 2019 23:47:40 +0000 Gerrit-HasComments: Yes
