Alexey Serbin has posted comments on this change.

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


Patch Set 6:

(5 comments)

Thanks for review!  Will post update soon.

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

Line 1000: # "make doxydocs" target
> Sorry to be a buzz kill, but can we just make this target "doxygen" instead
Sure, why not.  Will do.


Line 1015:     COMMAND make all install DESTDIR=${DOXY_CLIENT_DESTDIR}
> Can this be made to work with Ninja was well? I think you just need to repl
That's what I wondering as well, but I could not find it.  Thanks -- will 
replace with CMAKE_MAKE_PROGRAM.


http://gerrit.cloudera.org:8080/#/c/3619/6/docs/.gitignore
File docs/.gitignore:

Line 19
> nit: spurious change
Done


http://gerrit.cloudera.org:8080/#/c/3619/6/src/kudu/client/client.h
File src/kudu/client/client.h:

Line 28: #include <cstdint>
> This change isn't required for this patch, is it?
You are right -- it's not required.  Will rollback.


http://gerrit.cloudera.org:8080/#/c/3619/6/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:

Line 262: if ! `which -s doxygen` && [ ! -d $DOXYGEN_DIR ]; then
> The -s command-line flag is not available on GNU systems. You can do:
This is going away, actually.  I'll use the same approach as for the rest of 
the tools because of uniformity reasons.


-- 
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: 6
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: Jean-Daniel Cryans <jdcry...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-HasComments: Yes

Reply via email to