[GitHub] [tinkerpop] spmallette commented on issue #1122: Expose Connection Status to Client

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

spmallette commented on issue #1122: Expose Connection Status to Client
URL: https://github.com/apache/tinkerpop/pull/1122#issuecomment-498664697
 
 
   For now, I'll leave it to others to comment on the technical aspects of this pull request, but from an administrative point of view:
   
   1. Note that this PR points at the `master` branch which is now the 3.5.x line of code - thus, this may no longer be the desired target for this body of work. Perhaps it should be rebased to `tp34`
   1. The commit history is a bit muddy and includes a merge commit from `master`. Given the final disposition of the PR as it is now, the changeset is fairly small so and the commit history is mostly tweaks/cleanup/fixes/retries - it seems like it would be best to simply squash all of these commits to a single one.
   1. Not sure what others think but a JIRA issue for reference might be nice given the nature of the change. At a minimum though, we should have a [CHANGELOG entry](https://github.com/apache/tinkerpop/blob/master/CHANGELOG.asciidoc) and if "responsiveness" is that much better and this is deemed a worthy improvement that we should promote on release, then it would be nice to have some upgrade docs (in which case, I think creating an issue in JIRA is probably more necessary). 
   
   @github5775 perhaps it's smart not to act on any of these changes just yet. it's probably best to await an actual technical review of your changes before investing energy on these administrative items. thanks for contributing 

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