Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10342 )
Change subject: [flags] run validators after processing help flags ...................................................................... Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/10342/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10342/1//COMMIT_MSG@20 PS1, Line 20: linker optimizes the matter : and removes the validators if linking statically yea, unless you do something to enforce that the .o is loaded (eg add an empty function which is called from the master/tserver to ensure linking). Putting them into 'kserver' might make sense, though, since that's the module that already links into both? http://gerrit.cloudera.org:8080/#/c/10342/1//COMMIT_MSG@22 PS1, Line 22: kudu-tserver binary : should be enough. you aren't afraid of someone setting some crazy params on the master? http://gerrit.cloudera.org:8080/#/c/10342/1/src/kudu/integration-tests/CMakeLists.txt File src/kudu/integration-tests/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/10342/1/src/kudu/integration-tests/CMakeLists.txt@37 PS1, Line 37: tserver_proto we didn't actually depend on anything in the tserver module before? http://gerrit.cloudera.org:8080/#/c/10342/1/src/kudu/master/CMakeLists.txt File src/kudu/master/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/10342/1/src/kudu/master/CMakeLists.txt@84 PS1, Line 84: tserver this seems wrong -- To view, visit http://gerrit.cloudera.org:8080/10342 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I51f263890c91251f0d13c826c40dfd20fe284b59 Gerrit-Change-Number: 10342 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Tue, 08 May 2018 16:21:38 +0000 Gerrit-HasComments: Yes
