Matthew Jacobs has posted comments on this change. Change subject: Re-enable Kudu in build using client stubs when needed ......................................................................
Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/2585/2/CMakeLists.txt File CMakeLists.txt: Line 310: USE_KUDU If we're going to have this and the runtime flag, I think we need to disambiguate them a bit more. Use and enable sound too similar like options rather than what's actually supported. How about KUDU_CLIENT_SUPPORTED or KUDU_SUPPORTED? Line 320: find_package maybe just add a brief comment that this may be a stub client http://gerrit.cloudera.org:8080/#/c/2585/2/be/src/exec/kudu-util.h File be/src/exec/kudu-util.h: Line 30: Do we need all 3? Do we really need to know if it's just the first or second case, or can we always just call the method that returns the Status, and either use the status or just check if its OK/error? If we really need these calls, can you add a comment above explaining why we have them and when a caller would use one vs another? Line 32: Available I think we should avoid using Available for these flags since they mean different things and are confusing with the notion of availability, i.e. online-ness. Can we call this Supported? Line 36: Available Enabled? -- To view, visit http://gerrit.cloudera.org:8080/2585 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4bf1c964faf21722137adc4f7ba7f78654f0f712 Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Casey Ching <[email protected]> Gerrit-Reviewer: Casey Ching <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-HasComments: Yes
