[
https://issues.apache.org/jira/browse/TINKERPOP3-886?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15020228#comment-15020228
]
ASF GitHub Bot commented on TINKERPOP3-886:
-------------------------------------------
Github user spmallette commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/141#issuecomment-158584247
@kushal256 not a problem - i figured your IDE was the culprit. if you'd
like to amend your PR with the fix to the imports that will be fine.
The other thing we tend to do is mark all variables `final` (unless they
are not - which really makes them jump out at you and makes you wonder why they
are not when you see them in the code base). so for example you added:
```text
public static Io.Builder createIoBuilder(String graphFormat) throws
ClassNotFoundException, IllegalAccessException, InstantiationException {
Class<Io.Builder> ioBuilderClass = (Class<Io.Builder>)
Class.forName(graphFormat);
Io.Builder ioBuilder = ioBuilderClass.newInstance();
return ioBuilder;
}
```
`graphFormat`, `ioBuilder` and `ioBuilderClass` should be `final`.
`IoCore` is also a "public" API class, so some javadoc on that method would be
good.
And last but not least, if you want to really tie a bow on this, you would
add an entry to the CHANGELOG for 3.1.1:
https://github.com/apache/incubator-tinkerpop/blob/master/CHANGELOG.asciidoc#tinkerpop-311-not-officially-released-yet
Please comment when you've updated the PR with those little changes and
i'll merge after that.
> Allow any GraphReader/Writer to be persistence engine for TinkerGraph
> ----------------------------------------------------------------------
>
> Key: TINKERPOP3-886
> URL: https://issues.apache.org/jira/browse/TINKERPOP3-886
> Project: TinkerPop 3
> Issue Type: Improvement
> Components: tinkergraph
> Affects Versions: 3.0.2-incubating
> Reporter: stephen mallette
> Assignee: stephen mallette
> Priority: Trivial
>
> TinkerGraph currently works with gryo, graphml and graphson - all internal
> formats to TinkerPop. This could easily be extended to work with any format
> implementing the appropriate interfaces (i.e. also external third-party
> formats) by allowing for the `gremlin.tinkergraph.graphFormat` setting for
> TinkerGraph to be set to the existing three settings, but also allow for it
> to be the fully qualified class name for a {{Io.Builder<I extends Io>}}
> interface. TinkerGraph could then dynamically instantiate this class (which
> we can expect to have a zero-arg constructor) and use it to load/save data.
> This might actually also clean up some of the existing code around load/save.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)