You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by "nivsherf (GitHub)" <gi...@apache.org> on 2019/01/10 13:54:12 UTC
[GitHub] [tinkerpop] nivsherf opened pull request #1038:
TINKERPOP-2130 Fix default parameter assignment in Connection ctor
[ Full content available at: https://github.com/apache/tinkerpop/pull/1038 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1038:
TINKERPOP-2130 Fix default parameter assignment in Connection ctor
Posted by "spmallette (GitHub)" <gi...@apache.org>.
just peeking in on this PR - I'll let @jorgebay make the final word on this, but I guess the reason we needed to add this line was so that calls further down that use `options` and not `this.options` will work properly? shouldn't we just consistently use `this.options` rather than try to interchangeably use both as well as alter the state of the `options` parameter?
[ Full content available at: https://github.com/apache/tinkerpop/pull/1038 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] nivsherf commented on pull request #1038:
TINKERPOP-2130 Fix default parameter assignment in Connection ctor
Posted by "nivsherf (GitHub)" <gi...@apache.org>.
Makes sense @jorgebay. I updated the code accordingly.
[ Full content available at: https://github.com/apache/tinkerpop/pull/1038 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] jorgebay commented on issue #1038:
TINKERPOP-2130 Fix default parameter assignment in Connection ctor
Posted by "jorgebay (GitHub)" <gi...@apache.org>.
Thanks for the fix @nivsherf, VOTE +1
[ Full content available at: https://github.com/apache/tinkerpop/pull/1038 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] jorgebay commented on pull request #1038:
TINKERPOP-2130 Fix default parameter assignment in Connection ctor
Posted by "jorgebay (GitHub)" <gi...@apache.org>.
I think the safest bet would be to use double assignment, that way is concise and less error prone, dwyt @nivsherf ?
```javascript
this.options = options = options || {};
```
[ Full content available at: https://github.com/apache/tinkerpop/pull/1038 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz commented on issue #1038:
TINKERPOP-2130 Fix default parameter assignment in Connection ctor
Posted by "dkuppitz (GitHub)" <gi...@apache.org>.
VOTE +1
[ Full content available at: https://github.com/apache/tinkerpop/pull/1038 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] jorgebay closed pull request #1038:
TINKERPOP-2130 Fix default parameter assignment in Connection ctor
Posted by "jorgebay (GitHub)" <gi...@apache.org>.
[ pull request closed by jorgebay ]
[ Full content available at: https://github.com/apache/tinkerpop/pull/1038 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org