Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/10709 )
Change subject: IMPALA-7111: avoid use of boost::split in CheckPluginEnabled ...................................................................... Patch Set 1: (2 comments) Thanks for looking into this. I noticed that we also have gutil/strings/split.h in our sources. Should we use that for a simpler implementation? I don't feel strongly about it and am ok with the custom code, too. http://gerrit.cloudera.org:8080/#/c/10709/1/be/src/util/string-util-test.cc File be/src/util/string-util-test.cc: http://gerrit.cloudera.org:8080/#/c/10709/1/be/src/util/string-util-test.cc@84 PS1, Line 84: EXPECT_TRUE(CommaSeparatedContains("LZO", "LZO")); Should we have a test with different length in string and pattern, eg. "PLUGIN2", "LZO" and "A,B,C", "PLUGIN2"? http://gerrit.cloudera.org:8080/#/c/10709/1/be/src/util/string-util-test.cc@90 PS1, Line 90: EXPECT_FALSE(CommaSeparatedContains("", "LZO")); Nit: also test "," and ",,"? -- To view, visit http://gerrit.cloudera.org:8080/10709 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17b7f083731a33b832035f24900e351e2cb2feb8 Gerrit-Change-Number: 10709 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Comment-Date: Wed, 13 Jun 2018 21:11:59 +0000 Gerrit-HasComments: Yes
