Dinesh Bhat has posted comments on this change. Change subject: KUDU-1696. Daemons should dump their version info to the INFO log at startup ......................................................................
Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/4733/3/src/kudu/master/master_main.cc File src/kudu/master/master_main.cc: PS3, Line 62: executed Nit: master is yet to start execution, so I suggest re-wording to avoid confusion while reading the logs. "Master server non-default flags:\n" might also keep it unambiguous I guess. http://gerrit.cloudera.org:8080/#/c/4733/3/src/kudu/tserver/tablet_server_main.cc File src/kudu/tserver/tablet_server_main.cc: PS3, Line 58: executed Nit: same comment as in master_main.cc http://gerrit.cloudera.org:8080/#/c/4733/3/src/kudu/util/flags-test.cc File src/kudu/util/flags-test.cc: PS3, Line 85: "--flagfile=", Nit: we can expect the whole string provided at L75 I guess. PS3, Line 90: ndefault Is this a typo or did you intentionally put an invalid flagfile with 'ndefault' ? I was expecting 'test_default_ff' here. http://gerrit.cloudera.org:8080/#/c/4733/3/src/kudu/util/flags.cc File src/kudu/util/flags.cc: PS3, Line 360: google:: Also I think namespace prefixes 'std::' and 'google::' are redundant since we have them as header inclusions and forward-decl'ed. -- To view, visit http://gerrit.cloudera.org:8080/4733 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I846827c53e74ca364af61c690bf85af8a8f28601 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Maxim Smyatkin <smyatkinma...@gmail.com> Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
