Alexey Serbin 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 em Right: I also looked at the possibility to just provide the validator function in the library and the register the group validator in the master/tserver binary. It looked ugly to me. Yea, I look at base_server and other libraries to put that into. For some reason I missed kserver :( Indeed, kserver looks good! Thank you for pointing at it. 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? Yep, I'm a bit worried, and I was not happy with the code I posted, frankly. But I wanted to put something up to the review ASAP because of the Jepsen job failures. 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? It seems like that. Those changes came up while I was trying to chop off tserver and master libs dependency from the kudu CLI tool (but eventually it ended up in using mini-cluster which depends on the master and the tserver libs anyway). 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 Woops, that's some remnant from my experiments. -- 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: 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 18:02:27 +0000 Gerrit-HasComments: Yes
