Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19393 )
Change subject: [KUDU-3430] Implement getting flags filter logic in tool side ...................................................................... Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/19393/4/src/kudu/server/generic_service.cc File src/kudu/server/generic_service.cc: http://gerrit.cloudera.org:8080/#/c/19393/4/src/kudu/server/generic_service.cc@49 PS4, Line 49: gflag_filter > nit: What filter logic? Maybe have to mention 'gflag' and 'on server'? Done http://gerrit.cloudera.org:8080/#/c/19393/4/src/kudu/server/generic_service.cc@95 PS4, Line 95: if (!flags.empty() && !ContainsKey(flags, entry.first)) { : continue; : } : : bool matches = req->tags().empty(); : for (const auto& tag : req->tags()) { : > These code is duplicate, how about improved by: Done http://gerrit.cloudera.org:8080/#/c/19393/4/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/19393/4/src/kudu/tools/kudu-tool-test.cc@7817 PS4, Line 7817: public ToolTest, > What does 0 and 1 mean? How about use TEST_P instead of TEST_F in this case Done -- To view, visit http://gerrit.cloudera.org:8080/19393 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0a2ddc2540d7aa70d3dd2e6c4101bb227e94c858 Gerrit-Change-Number: 19393 Gerrit-PatchSet: 6 Gerrit-Owner: Wang Xixu <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Mon, 09 Jan 2023 10:22:36 +0000 Gerrit-HasComments: Yes
