Will Berkeley has posted comments on this change.

Change subject: [c++client/samples] added README for the sample
......................................................................


Patch Set 3:

(23 comments)

Some style, wording, and grammar nits. Don't have the chance to test out the 
instructions atm.

http://gerrit.cloudera.org:8080/#/c/4517/3/src/kudu/client/samples/README.adoc
File src/kudu/client/samples/README.adoc:

PS3, Line 6: of
remove extra "of"


PS3, Line 7:  
add "the"


PS3, Line 7: demonstates
nit: missing r "demonstrates"


PS3, Line 7: which
nit: s/which/that/


PS3, Line 9:  
add "the"


PS3, Line 16: at Kudu's Web site
"on Kudu's website" or "on the Kudu website"


PS3, Line 17: at
I like "on" best here, but "for" or "in" work. "at" doesn't sound right to me.


PS3, Line 17: `kudu-client0`
"`kudu-client0`package" or something to identify what it is


PS3, Line 25:  
add "a"


PS3, Line 27: is also possible but it is not recommended
style nit: "is possible but is not recommended"


PS3, Line 28: level of
style nit: remove


PS3, Line 30: E.g.
nit: say "For example" when starting a sentence instead of e.g.


PS3, Line 39: Unpack the `sample.cc.gz` file:
I would just combine this step with the previous one, to make a single 
"navigate to the directory and unpack" step


PS3, Line 44: just use `debug` for `CMAKE_BUILD_TYPE` correspondingly
style nit: "use `debug` in place of `release` in the command below"


PS3, Line 59:  
add "one" or "you"-- I think "you" is more in keeping with the rest of the doc


PS3, Line 63: if building
s/if building/to build/


PS3, Line 65: the directory
nit: remove


PS3, Line 68: E.g.
style nit: as above, s/E.g./For example/


PS3, Line 68:  
add "an"


PS3, Line 75: E.g.
Same nit as above


PS3, Line 80: just use `debug` for `CMAKE_BUILD_TYPE` correspondingly
style nit: "use `debug` in place of `release` in the command below"


PS3, Line 85: and
extra "and"


Line 88: 
Could you add a sample invocation and indicate what the user should see if the 
sample runs successfully?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d75340b2690663b56082de590cf19ea315da4fe
Gerrit-PatchSet: 3
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: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to