You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by "thefliik (GitHub)" <gi...@apache.org> on 2019/02/05 20:36:31 UTC
[GitHub] [tinkerpop] thefliik opened pull request #1056: fix:
Path#toString() variable reference bug
I'm going through the process of creating typescript definitions for the javascript library, and I noticed a bug in Path#toString(). It incorrectly attempts to call a non-existant local `objects` variable instead of calling the class property `this.objects`.
[ Full content available at: https://github.com/apache/tinkerpop/pull/1056 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] thefliik commented on issue #1056: fix:
Path#toString() variable reference bug
Posted by "thefliik (GitHub)" <gi...@apache.org>.
The code in question on `tp33`:
https://github.com/apache/tinkerpop/blob/tp33/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/graph.js#L118-L139
[ Full content available at: https://github.com/apache/tinkerpop/pull/1056 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] thefliik commented on issue #1056: fix:
Path#toString() variable reference bug
Posted by "thefliik (GitHub)" <gi...@apache.org>.
Seperately, would you be interested in a PR to port the whole javascript library over to typescript?
[ Full content available at: https://github.com/apache/tinkerpop/pull/1056 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1056: fix:
Path#toString() variable reference bug
Posted by "spmallette (GitHub)" <gi...@apache.org>.
`tp33` continually merges to `master` - wonder why `toString()` isn't present on that branch....
[ Full content available at: https://github.com/apache/tinkerpop/pull/1056 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] thefliik commented on issue #1056: fix:
Path#toString() variable reference bug
Posted by "thefliik (GitHub)" <gi...@apache.org>.
@jorgebay in the `tp33` branch it looks like the Path#toString() method has been removed entirely, so when/if that branch is merged into master this PR will be unnecessary. Feel free to close.
[ Full content available at: https://github.com/apache/tinkerpop/pull/1056 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1056: fix:
Path#toString() variable reference bug
Posted by "spmallette (GitHub)" <gi...@apache.org>.
Adding typescript definitions is being discussed here:
https://issues.apache.org/jira/browse/TINKERPOP-2027
perhaps you could read through that and include any thoughts you might have. i don't think anyone is working on that issue at this time, but you might want to ask on that in any comments you provide.
[ Full content available at: https://github.com/apache/tinkerpop/pull/1056 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] jorgebay commented on issue #1056: fix:
Path#toString() variable reference bug
Posted by "jorgebay (GitHub)" <gi...@apache.org>.
Thanks @thefliik , I've filed a ticket to keep track of this: https://issues.apache.org/jira/browse/TINKERPOP-2152
Can you target branch `tp33` for this to be applied to the 3.3 line?
[ Full content available at: https://github.com/apache/tinkerpop/pull/1056 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette closed pull request #1056: fix:
Path#toString() variable reference bug
Posted by "spmallette (GitHub)" <gi...@apache.org>.
[ pull request closed by spmallette ]
[ Full content available at: https://github.com/apache/tinkerpop/pull/1056 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] jorgebay commented on issue #1056: fix:
Path#toString() variable reference bug
Posted by "jorgebay (GitHub)" <gi...@apache.org>.
`toString()` was introduced as part of TK-1446, it didn't have a string representation on the js side prior to that.
@thefliik sorry for the confusion, nvm.
Fixing it on `master` makes sense, VOTE +1
[ Full content available at: https://github.com/apache/tinkerpop/pull/1056 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org