You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by Marko Rodriguez <ok...@gmail.com> on 2015/09/02 14:04:13 UTC
Use IDs and not just do ComputeVerificationStrategy Exception
Hello,
I just noticed this was done in tp30/ (I think @dkuppitz):
https://github.com/apache/incubator-tinkerpop/blob/tp30/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ComputerVerificationStrategy.java#L84-L88
This is not good. Why not just make it work. Like this (in master/):
https://github.com/apache/incubator-tinkerpop/blob/master/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GraphStep.java#L84-L93
That is, convert the elements to ids. ??
Please cherry pick the master/ work to tp30/ accordingly as to propagate the ComputerVerificationStrategy exception up the line might make us forget its solved...
Thanks,
Marko.
http://markorodriguez.com
Re: Use IDs and not just do ComputeVerificationStrategy Exception
Posted by Stephen Mallette <sp...@gmail.com>.
yeah - it needs to be right. i don't know why i didn't think of suggesting
that to him yesterday - was thinking too much about release stuff i guess.
the first fix for 3.0.2 has Mr.Kuppitz's name on it. :)
On Wed, Sep 2, 2015 at 8:16 AM, Marko Rodriguez <ok...@gmail.com>
wrote:
> Its not critical, its just pointless to not do it right.
>
> I would recommend that lines 84-88 get removed for 3.0.2 and the GraphStep
> solution in master/ get cherry picked down.
>
> Thanks Stephen for handling the release,
> Marko.
>
> http://markorodriguez.com
>
> On Sep 2, 2015, at 6:14 AM, Stephen Mallette <sp...@gmail.com> wrote:
>
> > I'm in mid-release mode for 3.0.1.....
> >
> > is it critical to include this? it was already an existing bug prior to
> > this and not something introduced by his work afaik.......
> >
> > On Wed, Sep 2, 2015 at 8:04 AM, Marko Rodriguez <ok...@gmail.com>
> > wrote:
> >
> >> Hello,
> >>
> >> I just noticed this was done in tp30/ (I think @dkuppitz):
> >>
> >>
> >>
> https://github.com/apache/incubator-tinkerpop/blob/tp30/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ComputerVerificationStrategy.java#L84-L88
> >>
> >> This is not good. Why not just make it work. Like this (in master/):
> >>
> >>
> >>
> https://github.com/apache/incubator-tinkerpop/blob/master/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GraphStep.java#L84-L93
> >>
> >> That is, convert the elements to ids. ??
> >>
> >> Please cherry pick the master/ work to tp30/ accordingly as to propagate
> >> the ComputerVerificationStrategy exception up the line might make us
> forget
> >> its solved...
> >>
> >> Thanks,
> >> Marko.
> >>
> >> http://markorodriguez.com
> >>
> >>
>
>
Re: Use IDs and not just do ComputeVerificationStrategy Exception
Posted by Marko Rodriguez <ok...@gmail.com>.
Its not critical, its just pointless to not do it right.
I would recommend that lines 84-88 get removed for 3.0.2 and the GraphStep solution in master/ get cherry picked down.
Thanks Stephen for handling the release,
Marko.
http://markorodriguez.com
On Sep 2, 2015, at 6:14 AM, Stephen Mallette <sp...@gmail.com> wrote:
> I'm in mid-release mode for 3.0.1.....
>
> is it critical to include this? it was already an existing bug prior to
> this and not something introduced by his work afaik.......
>
> On Wed, Sep 2, 2015 at 8:04 AM, Marko Rodriguez <ok...@gmail.com>
> wrote:
>
>> Hello,
>>
>> I just noticed this was done in tp30/ (I think @dkuppitz):
>>
>>
>> https://github.com/apache/incubator-tinkerpop/blob/tp30/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ComputerVerificationStrategy.java#L84-L88
>>
>> This is not good. Why not just make it work. Like this (in master/):
>>
>>
>> https://github.com/apache/incubator-tinkerpop/blob/master/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GraphStep.java#L84-L93
>>
>> That is, convert the elements to ids. ??
>>
>> Please cherry pick the master/ work to tp30/ accordingly as to propagate
>> the ComputerVerificationStrategy exception up the line might make us forget
>> its solved...
>>
>> Thanks,
>> Marko.
>>
>> http://markorodriguez.com
>>
>>
Re: Use IDs and not just do ComputeVerificationStrategy Exception
Posted by Stephen Mallette <sp...@gmail.com>.
sorry - accidentally hit "send" - the "bug" also has a clear workaround in
that the user can pass the id so it is non-blocking to usage. should I
still rollback release work?
On Wed, Sep 2, 2015 at 8:14 AM, Stephen Mallette <sp...@gmail.com>
wrote:
> I'm in mid-release mode for 3.0.1.....
>
> is it critical to include this? it was already an existing bug prior to
> this and not something introduced by his work afaik.......
>
> On Wed, Sep 2, 2015 at 8:04 AM, Marko Rodriguez <ok...@gmail.com>
> wrote:
>
>> Hello,
>>
>> I just noticed this was done in tp30/ (I think @dkuppitz):
>>
>>
>> https://github.com/apache/incubator-tinkerpop/blob/tp30/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ComputerVerificationStrategy.java#L84-L88
>>
>> This is not good. Why not just make it work. Like this (in master/):
>>
>>
>> https://github.com/apache/incubator-tinkerpop/blob/master/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GraphStep.java#L84-L93
>>
>> That is, convert the elements to ids. ??
>>
>> Please cherry pick the master/ work to tp30/ accordingly as to propagate
>> the ComputerVerificationStrategy exception up the line might make us forget
>> its solved...
>>
>> Thanks,
>> Marko.
>>
>> http://markorodriguez.com
>>
>>
>
Re: Use IDs and not just do ComputeVerificationStrategy Exception
Posted by Stephen Mallette <sp...@gmail.com>.
I'm in mid-release mode for 3.0.1.....
is it critical to include this? it was already an existing bug prior to
this and not something introduced by his work afaik.......
On Wed, Sep 2, 2015 at 8:04 AM, Marko Rodriguez <ok...@gmail.com>
wrote:
> Hello,
>
> I just noticed this was done in tp30/ (I think @dkuppitz):
>
>
> https://github.com/apache/incubator-tinkerpop/blob/tp30/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ComputerVerificationStrategy.java#L84-L88
>
> This is not good. Why not just make it work. Like this (in master/):
>
>
> https://github.com/apache/incubator-tinkerpop/blob/master/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GraphStep.java#L84-L93
>
> That is, convert the elements to ids. ??
>
> Please cherry pick the master/ work to tp30/ accordingly as to propagate
> the ComputerVerificationStrategy exception up the line might make us forget
> its solved...
>
> Thanks,
> Marko.
>
> http://markorodriguez.com
>
>