Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/569 Nice to see that all the tests are passing. Thanks for doing that. I've given this a quick review and have a few top-level comments: * Note that our code style is to explicitly use `final` for all variable declarations that require it - i think we make exceptions on exceptions in catch blocks and the var declaration of a `for()` loop, but that's about it. I think you've missed a few of those. * Could you please rename `BasicGraphManager` to `DefaultGraphManager`? We only use the term "Basic" in one other class naming, so it's a bit of an anomaly compared to "Default". * Does this change introduce any compile-time breaking changes for anyone who has depended on the `GraphManager` class (or other things you've modified)? I'm wondering if this is a change better suited for the master branch and 3.3.0. If it stays on `tp32` I may have to ask you to provide a second pull request to master that can also be evaluated as part of a second review/vote. * The addition of `addGraph()` is nice. * Can you elaborate on the usage of `openGraph()` - I think I get it, but it's not clear to me based on the javadoc/default implementation * Please add a entry (or more) to CHANGELOG for these updates. * Please update the reference documentation to include your new Gremlin Server configuration option here: http://tinkerpop.apache.org/docs/current/reference/#_configuring_2 - not sure how much you need to write here. Users of `GraphManager` have likely dug pretty deep into the code and will see how it works. Perhaps include a link to the javadoc for `GraphManager` and beef up the class javadoc there so that implementers know what to do. Just a bit of warning - from my perspective, this is a fairly involved pull request you've started (and thanks!) so it will likely take a some time and iterations to get through before we can get it merged.
--- 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. ---