[GitHub] tinkerpop issue #450: TINKERPOP-1489 Javascript GLV

[okram <gi...@git.apache.org>]

Github user okram commented on the issue:

    https://github.com/apache/tinkerpop/pull/450
  
    I did a preliminary review of the code and it looks good. Primarily because it mimics the structure and content of Gremlin-Python. Ensuring consistency between all variants is important from a maintainability standpoint.
    
    Here are some concerns:
    
    1. I think we need a `RemoteConnection` implementation. From what I'm reading, it seems we don't have one.
    2. @mbroecheler was pushing for async methods in Gremlin-Java (thus, "Gremlin"). The idea was to have `Future<V> Traversal.async(Function<Traversal,V>)`. Thus, to do a "future" `toList()` in Java, you would do `result = g.V().out().async(Traversal::toList)`. I think we should do this in Gremlin-Java and then have this same "callback" model used by Gremlin-JS and thus, not have `one()` and `list()` with callbacks. Does that make sense?
    3. I don't see the Gremlin `ProcessTestSuite` being run. We will need `Providers` to do so. Please see how this works in Gremlin-Python. https://github.com/apache/tinkerpop/tree/master/gremlin-python/src/test/java/org/apache/tinkerpop/gremlin/python/jsr223.
    4. We will need documentation in `gremlin-variants.asciidoc`.
    
    I suspect we will want to merge this first into an Apache TinkerPop branch and can nit pick things as I see them before an ultimate merge into `master/`. For instance, I can do 3 and 4 above if perhaps @jorgebay provides some notes so I get the important aspects in the documentation.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---