spmallette commented on pull request #1574:
URL: https://github.com/apache/tinkerpop/pull/1574#issuecomment-1065043996


   > Add queries in process suite integration tests for these steps.
   
   I'd agree that `element()` should probably have at least some process suite 
tests. doing that for `call()` seems somewhat questionable since they are a bit 
TinkerGraph specific, right (I almost wonder if they need to be in the Gherkin 
suite at all and not just live in `tinkergraph-gremlin`? 
   
   by 3.7.0 hopefully we can just be on gherkin and not have to duplicate tests 
anymore.
   
   > Add the new types such a ServiceRegistry to the serializers. A general 
question I have is around how the serializers (particularily graphbinary) will 
recognize the new tokens added in this reviews.
   
   I don't think we need serializers added, Maybe I'm not thinking of something 
but users are not sending `SeviceRegistry` instances back and forth during 
remote Gremlin calls. It's like JanusGraph users not working with remote 
instances of the management API. 
   
   There might be some need to add these classes for Gryo to suit OLAP sorts of 
cases. I assume `call()` will work fine in OLAP...we should probably test to be 
sure and if it doesn't document it. A test would quickly demonstrate if 
serialization uses cases aren't happy.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to