Github user bzz commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/869#issuecomment-216088248
@hriviere thank you, great work!
Couple of small things:
- could you please make sure that all code and `pom.xml` formatting is
consistent [project's style
guide](https://github.com/apache/incubator-zeppelin/blob/master/CONTRIBUTING.md#code-convention)?
- in TODOs I see you plan to add simple test for this interpreter, right?
It will be really great to have at least one or two, like there are in
[other](https://github.com/apache/incubator-zeppelin/blob/master/spark/src/test/java/org/apache/zeppelin/spark/SparkInterpreterTest.java#L168)
[interpreters](https://github.com/apache/incubator-zeppelin/blob/master/elasticsearch/src/test/java/org/apache/zeppelin/elasticsearch/ElasticsearchInterpreterTest.java#L143)
- also, as soon as `progress()` and `cancel()` are not supported for this
interpreter yet, do you think it might be worth mentioning it in the docs?
- as there most probably will be other people working in this code later
on, what do you think about adding `python/REAMDE.md` with a sentence about
high-level description of this interpreter architecture? Like it's
pre-requests, main idea and some very brief details i.e what Py4J is used for,
etc.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
