You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by "spmallette (GitHub)" <gi...@apache.org> on 2019/12/04 16:35:55 UTC

[GitHub] [tinkerpop] spmallette commented on issue #1224: TINKERPOP-1733 Support g.E().properties().hasKey('xx') & hasValue('xx')

wow - this is starting to look really concise. nice to see all the tests passing with your changes. that provides some nice confidence. i have a few thoughts at this point:

* Please add some CHANGELOG entries for the changes you've put in place.
* I think this is an important enough change that it requires some upgrade documentation to help summarize for users what they can expect as a result of these changes - you would add them [here](https://github.com/apache/tinkerpop/blob/tp34/docs/src/upgrade/release-3.4.x.asciidoc).
* I assume there are no breaking changes in existing behavior here (aside from behavior that was previously undesired) - please correct me if I'm wrong.

If you're really busy, I can probably add those changes on merge, but those doc fixes would make this PR look really complete to me. Assuming those things are all settled:

VOTE +1

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