You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@tinkerpop.apache.org by "joestrouth1 (GitHub)" <gi...@apache.org> on 2019/02/24 07:03:07 UTC

[GitHub] [tinkerpop] joestrouth1 opened pull request #1071: TINKERPOP-2167 Traversals as async iterables in gremlin-javascript

https://issues.apache.org/jira/browse/TINKERPOP-2167

Summary of changes:
- Added `[Symbol.asyncIterator]` member to the Traversal class to support `for await ... of` loops (async iterables)

Change was tested by modifying the latest published version of `gremlin` on npm. Reviewed current tests for `gremlin-javascript` and saw none that might conflict.

[ Full content available at: https://github.com/apache/tinkerpop/pull/1071 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] jorgebay commented on issue #1071: TINKERPOP-2167 Traversals as async iterables in gremlin-javascript

Posted by "jorgebay (GitHub)" <gi...@apache.org>.

thanks @spmallette !

[ Full content available at: https://github.com/apache/tinkerpop/pull/1071 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] jorgebay commented on issue #1071: TINKERPOP-2167 Traversals as async iterables in gremlin-javascript

Posted by "jorgebay (GitHub)" <gi...@apache.org>.

Thanks for your contribution @joestrouth1 ! Its a good idea to expose a `Symbol.asyncIterator` method.

We must support Node.js 6 and 8 runtimes, that don't expose `Symbol.asyncIterator`.
You can fix it by define it conditionally:

```javascript
const asyncIteratorSymbol = Symbol.asyncIterator || Symbol('@@asyncIterator');

// ...
  [asyncIteratorSymbol]() {
    // This wont work for Node.js 6 and 8, but at least the method will be exposed
    // under symbol name
    return this;
  }
```

Also, it would be nice it we could add a [unit test](https://github.com/apache/tinkerpop/blob/master/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/traversal-test.js) validating that `Symbol.asyncIterator` is defined for a `Traversal` instance:

```javascript
if (Symbol.asyncIterator) {
  describe('@@iterator', () => {
    it('should expose the async iterator');
  });
}
```

[ Full content available at: https://github.com/apache/tinkerpop/pull/1071 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] joestrouth1 commented on issue #1071: TINKERPOP-2167 Traversals as async iterables in gremlin-javascript

Posted by "joestrouth1 (GitHub)" <gi...@apache.org>.

Apologies for the delay! It's been hectic. Thanks for working around.

[ Full content available at: https://github.com/apache/tinkerpop/pull/1071 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] joestrouth1 commented on issue #1071: TINKERPOP-2167 Traversals as async iterables in gremlin-javascript

Posted by "joestrouth1 (GitHub)" <gi...@apache.org>.

@jorgebay Sure thing, I should be able to get to it this weekend. 

[ Full content available at: https://github.com/apache/tinkerpop/pull/1071 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] jorgebay commented on issue #1071: TINKERPOP-2167 Traversals as async iterables in gremlin-javascript

Posted by "jorgebay (GitHub)" <gi...@apache.org>.

@spmallette You are right, it does make sense to deliver it on `3.3.x`...

@joestrouth1 Do you mind basing your pull request on `tp33` so this can land on both `3.3` and `3.4`?
In case you don't have time to do it, I could cherry pick it.

[ Full content available at: https://github.com/apache/tinkerpop/pull/1071 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] spmallette commented on issue #1071: TINKERPOP-2167 Traversals as async iterables in gremlin-javascript

Posted by "spmallette (GitHub)" <gi...@apache.org>.

cherry-picked to `tp33` via 

f16b50f14b50dc1510889a30f1776e7ebe27b02f
d1b3da8ab36a244c88c710f8bbbe8c59cb748866
78d4e8248da98ffee507ac9c8961d75187eeccd0

and merged to master. thanks!

[ Full content available at: https://github.com/apache/tinkerpop/pull/1071 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] spmallette commented on issue #1071: TINKERPOP-2167 Traversals as async iterables in gremlin-javascript

Posted by "spmallette (GitHub)" <gi...@apache.org>.

@jorgebay are we ok with this only going to `master` / 3.4.1? also, is there any need for documentation changes in relation to this? finally, this needs a CHANGELOG entry which can be done by on merge i suppose.

other than that: VOTE +1 - thanks @joestrouth1 

[ Full content available at: https://github.com/apache/tinkerpop/pull/1071 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] spmallette closed pull request #1071: TINKERPOP-2167 Traversals as async iterables in gremlin-javascript

Posted by "spmallette (GitHub)" <gi...@apache.org>.

[ pull request closed by spmallette ]

[ Full content available at: https://github.com/apache/tinkerpop/pull/1071 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org