Github user okram commented on the issue:

    https://github.com/apache/incubator-tinkerpop/pull/325
  
    @dalaro -- your explanation of the problem in JIRA is impeccable. I now I 
remember why "just extending `SparkSerializer`" was not an option. TinkerPop's 
Kryo is shaded. Your concept of a shim to abstract both shaded Kryo to be used 
by unshaded Kryo systems is smart. Looking at the code specifically, 
unfortunately, its hard to review and understand the full ramifications of what 
you are providing. I can run the integration test suite and all that, but 
perhaps you could also answer some questions as well. 
    
    **Note that I haven't looked at this code in over 6 months so my questions 
may be a little odd sounding.**
    
    1. I never liked `HadoopPools`. I have a sneaking suspicion that they may 
not work with custom class registration and I we really should have a test case 
in there to prove yay or nay. Does your work either make `HadoopPools` "better" 
or no longer needed? (Also think about `GiraphGraphComputer` which uses 
`HadoopPools`).
    
    2. Moving forward, would people still use `GryoSerializer` or is it 
recommended that they use `SparkSerializer`? How would your work effect our 
docs and does `GryoSerializer` become deprecated?
    
    3. I noticed you moved around the serialization code for `StarGraph`. That 
is fine, but I'm wondering, did you change the serialization of `StarGraph`? 
That is, did the `byte[]` format change? If so, thats bad for backwards 
compatibility.
    
    Finally, your PR is beautiful. You JavaDoc like a king and you prove your 
worth as emperor with `package-info.java`. Thank you for your efforts in a part 
of the codebase that I wrote with a looming lack of confidence. You have 
brought a level of Java skill/technique to a long standing problem that is 
inspiring.


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

Reply via email to