Github user dimas-b commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/899#discussion_r206542682
--- Diff:
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
---
@@ -222,9 +223,33 @@ protected AbstractEvalOpProcessor(final boolean
manageTransactions) {
* script evaluation.
* @param bindingsSupplier A function that returns the {@link
Bindings} to provide to the
* {@link GremlinExecutor#eval} method.
+ * @see #evalOpInternal(ResponseHandlerContext, Supplier,
BindingSupplier)
*/
protected void evalOpInternal(final Context context, final
Supplier<GremlinExecutor> gremlinExecutorSupplier,
--- End diff --
Yeah, I was thinking about that but could not really reach any decision
myself, so I kept it more accessible. In current code no other classes need to
call the new method, so I'm ok to make it `private`.
However, if we keep the new `evalOpInternal` method `private` by the same
logic it might be reasonable to keep the new `makeFrame` method `private` as
well. If other classes ever need to use it directly it would be under similar
conditions as calling the new `evalOpInternal` method... and right now other
classes do not need to do that.
What do you think?
---