[GitHub] [tinkerpop] spmallette commented on issue #1105: TINKERPOP-2205 Change connection management to single request per channel

[GitBox <gi...@apache.org>]

spmallette commented on issue #1105: TINKERPOP-2205 Change connection management to single request per channel
URL: https://github.com/apache/tinkerpop/pull/1105#issuecomment-500579931
 
 
   Are there concerns with https://github.com/netty/netty/pull/9226 ? Is that a problem we introduce as a new exposed risk as a result of this PR or is it something that can happen now with how things work in `tp34`? 
   
   I think the code changes seems fine from the best I can tell by just scanning through. I had a few nits around the "deprecated" bits of code. If you could just do a quick pass through and add the appropriate javadoc in the form we use, that would be great. For future reference, here's some additional information in our dev docs on the topic:
   
   http://tinkerpop.apache.org/docs/current/dev/developer/#_deprecation
   
   No need to include the `@see` link everywhere to the JIRA i don't think....perhaps a strategically placed one somewhere would suffice. 
   
   I would really like to see the documentation changes to consider it all a "finished" PR though. Specifically I think I'd like to see:
   
   1. User Documentation - delete the deprecated settings from the docs and modify configuration/best practices sections as needed to reflect how the driver works now.
   2. User Upgrade Documentation - something to call attention to this change and explains what was done and why (much of that is here in your PR description so you could probably copy/paste a bit) as well as what users need to do on upgrade to be sure everything works well.
   3. Provider Upgrade Documentation - document any things that might possibly break as a result of this change (you mentioned "providers have provided a custom channelizer" for example).
   
   Starting the process of testing manually now. Assuming I don't run into any problems there, when you push the documentation changes, I think this PR will be basically good to go. Thanks.
   
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services