Adar Dembo has posted comments on this change. Change subject: doxygen for C++ client API ......................................................................
Patch Set 8: (5 comments) http://gerrit.cloudera.org:8080/#/c/3619/8/CMakeLists.txt File CMakeLists.txt: Line 983: set(DOXY_SUBDIR ${CMAKE_CURRENT_BINARY_DIR}/docs/doxygen) > The find_package(Doxygen) outputs information on location of doxygen binary Ah, great. I think that's sufficient. Line 989: add_custom_target(doxy_install_client_alt_destdir > The 'all' and 'install' steps are here is for kudu-client sub-dir, actually Maybe there's no real 'all' target after all. What about depending on 'kudu_client_exported'? That's the target that builds the client library. Line 990: COMMAND ${CMAKE_COMMAND} -E remove_directory ${DOXY_CLIENT_DESTDIR} > This is for cleaning some stale stuff if it's left around when make was int Hmm, OK. I was hoping that a second "make install" would automatically clear out the old state, but maybe it doesn't. Line 992: WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/src/kudu/client > Because we need only client-related parts to be installed into the alternat You don't have to worry about that, though; "make install" only installs the client library and headers. If/when it installs more (especially if "more" includes "things that might confuse doxygen"), we can further restrict Doxygen to run in ${DOXY_CLIENT_DESTDIR}/usr/include or some such. http://gerrit.cloudera.org:8080/#/c/3619/8/thirdparty/download-thirdparty.sh File thirdparty/download-thirdparty.sh: PS8, Line 63: if [[ "$FILENAME" =~ \.zip$ ]]; then : if ! unzip -q "$FILENAME"; then : echo "Error unzipping $FILENAME, removing file" : rm "$FILENAME" : continue : fi : elif [[ "$FILENAME" =~ \.(tar\.gz|tgz)$ ]]; then > This change allows avoid running extra-processes to check for regex pattern OK, I see. Let's keep them, then. I just wanted to make sure it wasn't a change from one personal style to another; we discourage such changes due to the inherent "ping-ponging" effect they can elicit. -- 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