Github user spmallette commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/245#issuecomment-190690137
Removing the `ServiceLoader` is a bigger change than it immediately
appears. The `ServiceLoader` provides functionality that allows the console to
show the user the list of plugins that are present but not "activated":
```text
gremlin> :plugin list
==>tinkerpop.server[active]
==>tinkerpop.gephi
==>tinkerpop.utilities[active]
==>tinkerpop.sugar
==>tinkerpop.credentials
==>tinkerpop.tinkergraph[active]
```
With this change in place you get:
```text
gremlin> :plugin list
==>tinkerpop.server[active]
==>tinkerpop.utilities[active]
==>tinkerpop.tinkergraph[active]
```
Of course, as we are learning, the notion of "active" is somewhat blurry
because an "inactive" plugin may still load some aspect of a class during
instantiation by `ServiceLoader`. An "inactive" plugin is really one that
doesn't have its plugin methods called to perform imports, init the console
environment, etc.
I guess the question is whether or not its valuable to know what plugins
are available but not active. If it was determined that we could just drop that
capability, there'd be some additional code/docs to amend to this PR. If we
wanted to try to keep this functionality, i guess a custom `ServiceLoader`
could work - it would just need to do the same thing as the current
`ServiceLoader` but not throw an exception if a class could not be found -
though that would have its own dangers in some sense.
@bdeggleston do you see any other ways to implement this without losing the
functionality of listing available plugins as it works now?
---
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.
---
