Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10582 )

Change subject: KUDU-2191: use 'kudu_only' in metadata upgrade tool
......................................................................


Patch Set 3:

> >  > The LSAN failure looks legit. Also, why does tool_action_hms
 > >  > include a test header (client-test-util.h)? Why is this new
 > >  > function defined in a test header?
 > >
 > > tool_action_hms includes client-test-util.h because it uses a
 > util method SchemaFromKuduSchema defined in client-test-util, but
 > maybe it would make more sense to move them all to sth like
 > client-util.h?
 >
 > Sure. Or maybe in schema-internal.h, which is currently used for
 > the internal definition of a client schema (i.e. KuduSchema::Data).
 > It's not a perfect fit but you won't need to create a new header.

Thanks for the suggestion! I updated this patch to define the new function
in client-internal.h. I will do another patch for the other method.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d
Gerrit-Change-Number: 10582
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Hao Hao <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 07 Jun 2018 19:30:56 +0000
Gerrit-HasComments: No

Reply via email to