Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/13294 )
Change subject: [util] Introduce yaml-cpp to read config files ...................................................................... Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/13294/5/cmake_modules/FindYaml.cmake File cmake_modules/FindYaml.cmake: http://gerrit.cloudera.org:8080/#/c/13294/5/cmake_modules/FindYaml.cmake@22 PS5, Line 22: find_library(YAML_STATIC_LIB libyaml-cpp.a > Kudu's default linkage is "automatic", which means dynamic linked binaries A very good advice, thanks! http://gerrit.cloudera.org:8080/#/c/13294/5/src/kudu/util/yamlreader.h File src/kudu/util/yamlreader.h: http://gerrit.cloudera.org:8080/#/c/13294/5/src/kudu/util/yamlreader.h@39 PS5, Line 39: // Wraps the YAML parsing functionality of YAML::Node. > Could you also doc the public methods? See jsonreader.h for an example. Done http://gerrit.cloudera.org:8080/#/c/13294/7/src/kudu/util/yamlreader.cc File src/kudu/util/yamlreader.cc: http://gerrit.cloudera.org:8080/#/c/13294/7/src/kudu/util/yamlreader.cc@73 PS7, Line 73: return Status::NotFound(Substitute("parse field $0 error: $1", field, e.what())); > Does yaml-cpp offer finer grained errors or exception types that we can map Seems not. yaml-cpp doesn't offer any error codes, but offer some exceptions, but most of the exceptions throwed are very general, like InvalidNode, ParserException, not grained enough to map to Status in Kudu. -- To view, visit http://gerrit.cloudera.org:8080/13294 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8ef58befaffbcc880e13fa6fec61b8e94a189b5a Gerrit-Change-Number: 13294 Gerrit-PatchSet: 8 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Wed, 15 May 2019 06:35:44 +0000 Gerrit-HasComments: Yes
