Github user davebshow commented on the issue:

    https://github.com/apache/tinkerpop/pull/448
  
    I like this. Much simpler, well tested, in general a nice PR. Pretty slick 
using the metaclass to create the de-/serializer maps. One thing I wonder: 
    
    Presumably using the `GraphSONIO` instance methods `writeObject`/`toObject` 
methods instead of the previous classmethods provides an advantage because we 
can now pass a custom serializer/deserializer map to the `GraphSONIO` instance, 
thus overriding the default serializers. However, the `GraphSONIO` instance is 
created inside the `DriverRemoteConnection` without the optional kwargs. I 
wonder if we should add `graphson_io=None` to the `__init__` method signature. 
In the case of `None`, we can instantiate the object as is:
    
    ```
    def __init__(self, url, traversal_source, username="", password="", 
loop=None, graphson_io=None):
        if not graphson_io:
            graphson_io = GraphSONIO()
        self._graphson_io = graphson_io
    ```
    
    Other than that and my previous comment. This is LGTM.
    
    VOTE + 1


---
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.
---

Reply via email to