Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14329 )
Change subject: [java] KUDU-2971: process communicates via protobuf-based protocol ...................................................................... Patch Set 19: (1 comment) http://gerrit.cloudera.org:8080/#/c/14329/17/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java File java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java: http://gerrit.cloudera.org:8080/#/c/14329/17/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java@33 PS17, Line 33: public class SubprocessConfiguration { > I want to avoid adding inline comment /*blah=*/, also some of the variables But with the exception of the boolean, these are all pretty self explanatory. At least, looking at how we define such options and descriptions in the C++ tooling, defining constants for these seems like overkill. And even if we did want to check the strings match in tests, we usually favor matching on the string literal rather than a constant (see many of our C++ tooling tests for examples) -- we're testing that we get a message that makes sense for the test, not that we're using a specific string variable. -- To view, visit http://gerrit.cloudera.org:8080/14329 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaf9ad24dbc9acc681284b6433836271b5b4c7982 Gerrit-Change-Number: 14329 Gerrit-PatchSet: 19 Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 07 Feb 2020 07:49:53 +0000 Gerrit-HasComments: Yes
