Jordan Birdsell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10794 )

Change subject: [examples] KUDU-2382: Add Kudu Basic Python Example
......................................................................


Patch Set 2:

(12 comments)

I added some basic documentation in the readme on how to install kudu (from 
source or from pypi).  I think eventually we also want this documented 
elsewhere.

http://gerrit.cloudera.org:8080/#/c/10794/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10794/1//COMMIT_MSG@7
PS1, Line 7: [examples] KUDU-2382: 
> nit: [examples] KUDU-2382: ...
Done


http://gerrit.cloudera.org:8080/#/c/10794/1//COMMIT_MSG@10
PS1, Line 10:
> nit: is
Done


http://gerrit.cloudera.org:8080/#/c/10794/1/examples/python/basic-python-example/README.md
File examples/python/basic-python-example/README.md:

http://gerrit.cloudera.org:8080/#/c/10794/1/examples/python/basic-python-example/README.md@2
PS1, Line 2: Licensed under the Apache License, Version 2.0
> Why does this license header differ from our standard one? Because it's not
Not sure, i copied this from one of the examples directories. Want me to change 
it?


http://gerrit.cloudera.org:8080/#/c/10794/1/examples/python/basic-python-example/basic_example.py
File examples/python/basic-python-example/basic_example.py:

http://gerrit.cloudera.org:8080/#/c/10794/1/examples/python/basic-python-example/basic_example.py@23
PS1, Line 23:
> This needs to be configurable so the example is runnable.
Done


http://gerrit.cloudera.org:8080/#/c/10794/1/examples/python/basic-python-example/basic_example.py@25
PS1, Line 25: parser.add_argument('--masters', '-m', nargs='+', 
default='localhost',
> nit: End comments with a . here and below.
Done


http://gerrit.cloudera.org:8080/#/c/10794/1/examples/python/basic-python-example/basic_example.py@31
PS1, Line 31:
> nit: add "the" or "its"
Done


http://gerrit.cloudera.org:8080/#/c/10794/1/examples/python/basic-python-example/basic_example.py@34
PS1, Line 34:
> nit: a
Done


http://gerrit.cloudera.org:8080/#/c/10794/1/examples/python/basic-python-example/basic_example.py@51
PS1, Line 51: Open a t
> nit: Update
Done


http://gerrit.cloudera.org:8080/#/c/10794/1/examples/python/basic-python-example/basic_example.py@59
PS1, Line 59: 
> nit: extra word
Done


http://gerrit.cloudera.org:8080/#/c/10794/1/examples/python/basic-python-example/basic_example.py@62
PS1, Line 62:
> Is there anything worth printing in this object?
No, removed it


http://gerrit.cloudera.org:8080/#/c/10794/1/examples/python/basic-python-example/basic_example.py@69
PS1, Line 69: e a row
> nit: scanner
Done


http://gerrit.cloudera.org:8080/#/c/10794/1/examples/python/basic-python-example/basic_example.py@71
PS1, Line 71: session.apply(op)
> We should print and/or verify the results are as expected.
I added an example to print it and put a comment telling the user what the 
expected output is. I didn't think an assert would be all that valuable.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12016a7c9b1f4a1e20556297654c7abfa7b673cb
Gerrit-Change-Number: 10794
Gerrit-PatchSet: 2
Gerrit-Owner: Jordan Birdsell <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Jordan Birdsell <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <[email protected]>
Gerrit-Comment-Date: Sun, 24 Jun 2018 17:32:16 +0000
Gerrit-HasComments: Yes

Reply via email to