[GitHub] tinkerpop issue #816: TINKERPOP-1919 Merge classes P and TraversalPredicate ...
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/816 > Although it will probably just concentrate on cases where users already implemented their own predicates as there should be no visible changes for users that only used our predicates exactly - VOTE +1 ---
[GitHub] tinkerpop issue #816: TINKERPOP-1919 Merge classes P and TraversalPredicate ...
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/816 >I think this is a good change though it doesn't exactly mimic the Java API right? there is no until(P) in java - it's until(Predicate), but I guess that doesn't really make sense in this GLV to support something like that because we can really only serialize values of P across to the server. This will be changed with PR #815 that introduces new interfaces, one of which is `IPredicate` that we use in Gremlin.Net in places where Java uses `Predicate`. My idea was that users could write either predicates in the form of lambdas (PR #814) or implement their own predicates which would also require their own serialization. Geo-predicates or full-text search are an example where implementing custom predicates like this would make sense. I'll add something for the upgrade docs. Although it will probably just concentrate on cases where users already implemented their own predicates as there should be no visible changes for users that only used our predicates. `P.gt(1).and(P.lt(3))` can still be called exactly the same way. The only difference is that they now get back a `P` object instead of a `TraversalPredicate`. ---
[GitHub] tinkerpop issue #816: TINKERPOP-1919 Merge classes P and TraversalPredicate ...
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/816 I think this is a good change though it doesn't exactly mimic the Java API right? there is no `until(P)` in java - it's `until(Predicate)`, but I guess that doesn't really make sense in this GLV to support something like that because we can really only serialize values of `P` across to the server. I agree that most folks will not have used this `TraversalPredicate` aspect of the API, so I agree that we can allow this breaking change to occur. However, I think that you should call more attention to it by adding something to the upgrade docs that describes the change and shows what the code used to look like and what it should look like now. Does all of that make sense? ---
[GitHub] tinkerpop issue #816: TINKERPOP-1919 Merge classes P and TraversalPredicate ...
Github user jorgebay commented on the issue: https://github.com/apache/tinkerpop/pull/816 Down to just 32 ignored! VOTE +1 ---
