Adar Dembo has posted comments on this change.

Change subject: doxygen for C++ client API
......................................................................


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3619/8/CMakeLists.txt
File CMakeLists.txt:

Line 989:     add_custom_target(doxy_install_client_alt_destdir
> Yesterday I tried dependency on 'kudu_client_exported' as well :)  The prob
Interesting. I think the kudu_client_exported problem is due a misuse of 
add_dependencies(), where the gen_version_info target is set as a dependency of 
kudu_util but not kudu_util_exported.

Can you try this patch? It should allow depending on kudu_client_exported 
properly, and it's a good improvement to make regardless.

  diff --git a/src/kudu/util/CMakeLists.txt b/src/kudu/util/CMakeLists.txt
  index a924f38..df1b4c9 100644
  --- a/src/kudu/util/CMakeLists.txt
  +++ b/src/kudu/util/CMakeLists.txt
  @@ -218,10 +218,9 @@ endif()
   ADD_EXPORTABLE_LIBRARY(kudu_util
     SRCS ${UTIL_SRCS}
     DEPS ${UTIL_LIBS}
  +  NONLINK_DEPS gen_version_info
     EXPORTED_DEPS ${EXPORTED_UTIL_LIBS})
 
  -add_dependencies(kudu_util gen_version_info)
  -
   #######################################
   # kudu_test_util
   #######################################


Line 992:       WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/src/kudu/client
> Yes, but why to depend on that at all and spend extra-time fixing that if s
Running make out of anywhere but the top-level directory isn't something most 
(if any) of us do, so I'm not sure how much I trust it. Plus, it means another 
place to update if we wanted to move the client code (or the client's install 
code) to a different directory. In that sense, the target abstracts away the 
filesystem layout, and that's a good thing.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-HasComments: Yes

Reply via email to