Github user jbmusso commented on the issue:
https://github.com/apache/tinkerpop/pull/695
Finally found some time. Wew.
Well, this PR is very well crafted - well done! My comments are only minor:
* About `utils.toPromise` - if I understand you well, you want a dual
callback/promise API for most async functions?
* In `graph-traversal.js`, most methods of `GraphTraversalSource` and
`GraphTraversal` and function attached to `statics` could maybe be dynamically
created from an array of method names and dynamically added to these
classes/object. I donât know what would be the performance implication of
this, but I donât think there should be any unless V8 really canât figure
out whatâs going on when parsing that file. Hopefully it's smart and figures
out that the class is not changing. Thatâd lower the file size and help
maintainability a lot.
* ES6, most likely friendlier for more recent versions of V8 and supported
by Node.js v4+ (see [Node green](http://node.green)):
* `arguments` is deprecated and is replaced by `...args` for variadic
functions
* most `function` keywords could be replaced by arrow functions
(lexical scoping and/or concise syntax). I tend to keep `function` for
top-level functions, and use fat arrows everywhere even when `this` binding
isn't needed (ie. callback w/o `this`)
* `array.splice(0)` could be replaced by `const copy = [...original]`
* `func.apply(null, arguments)` could be replaced by `func(...args)`
when first argument `this` value is indeed meant to be `null`
* `package.json`: `./node_modules/.bin` is added to the `$PATH` by `npm` or
`yarn`, so we could just use `"test": "mocha test/unit test/integration -t
5000"`. Yay npm!
I can fork and push 4 distinct commits for this if you want, so this can be
cherry-picked.
A more major update would be to author in ES6/7/8 and add a transpilation
step, so all runtime could use code that they can optimize best. Using babel
with [babel-preset-env](https://www.npmjs.com/package/babel-preset-env)
combined with
[postinstall-build](https://www.npmjs.com/package/postinstall-build) is an
option. This will ensure that latest versions of Node.js use mostly
non-transpiled code, while older versions automatically transpile what is
strictly needed. That would make things more performant for latest versions of
Node.js, since V8 optimizes a lot for new syntax, while still making the GLV
compatible for older versions of Node.js. The nice thing is that the build step
is automatically handled at install time by npm, so no extra maven coding
should be required. I think such approach could be added in future releases and
is out of scope today.
Also, I'm ok to transfer/donate the "gremlin" package name to Apache
TinkerPop so this can be published under this name.
---
