Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9462 )

Change subject: Make metrics name matching case-insensitive
......................................................................


Patch Set 1:

(2 comments)

Code looks fine but what's the motivation for this change?

http://gerrit.cloudera.org:8080/#/c/9462/1/src/kudu/util/metrics-test.cc
File src/kudu/util/metrics-test.cc:

http://gerrit.cloudera.org:8080/#/c/9462/1/src/kudu/util/metrics-test.cc@214
PS1, Line 214:   // Verify that filtering is case-insensitive.
While you're there, maybe also test that it's indeed a substring match?


http://gerrit.cloudera.org:8080/#/c/9462/1/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:

http://gerrit.cloudera.org:8080/#/c/9462/1/src/kudu/util/metrics.cc@194
PS1, Line 194:   string param_uc;
Nit: could be declared inside the loop.



--
To view, visit http://gerrit.cloudera.org:8080/9462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
Gerrit-Change-Number: 9462
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wdberke...@gmail.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com>
Gerrit-Comment-Date: Fri, 02 Mar 2018 19:38:17 +0000
Gerrit-HasComments: Yes

Reply via email to