[GitHub] [tinkerpop] spmallette opened pull request #1211: TINKERPOP-1568 Changed Strategy Application Methodology

["spmallette (GitHub)" <gi...@apache.org>]

https://issues.apache.org/jira/browse/TINKERPOP-1568

Please refer to the JIRA issue for details as well as the Upgrade Docs on this PR for more information about this one. I'm generally happy with how few changes were actually involved in making this happen - if you take away my light refactoring for inclusion of the `IsRoot()` method very few files had to really change and for the most part the changes themselves were fairly straightforward. It didn't always look this way - took a fair amount of effort to get to the root of why certain changes were necessary to then refactor for greater simplicity. 

I think the only thing I wasn't really able to sort out "nicely" was `MessagePassingReductionStrategy`. It seems to need to know a lot about `profile()` for some reason and I wasn't quite able to sort out a better way to do what was necessary there. I think I'm just going to have to submit that as-is unless someone has better ideas.

All tests pass with `docker/build.sh -t -n -i`

VOTE +1

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

[GitHub] [tinkerpop] spmallette commented on pull request #1211: TINKERPOP-1568 Changed Strategy Application Methodology

["spmallette (GitHub)" <gi...@apache.org>]

that can be investigated: https://issues.apache.org/jira/browse/TINKERPOP-2310

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

[GitHub] [tinkerpop] jorgebay commented on issue #1211: TINKERPOP-1568 Changed Strategy Application Methodology

["jorgebay (GitHub)" <gi...@apache.org>]

Nice!
I'll be able to review it by early next week.

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

[GitHub] [tinkerpop] jorgebay commented on pull request #1211: TINKERPOP-1568 Changed Strategy Application Methodology

["jorgebay (GitHub)" <gi...@apache.org>]

ah, I see...

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

[GitHub] [tinkerpop] jorgebay commented on pull request #1211: TINKERPOP-1568 Changed Strategy Application Methodology

["jorgebay (GitHub)" <gi...@apache.org>]

I think this is a sign that `TraversalStrategies` should extend `Iterable<TraversalStrategy<?>>` instead.

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

[GitHub] [tinkerpop] jorgebay commented on pull request #1211: TINKERPOP-1568 Changed Strategy Application Methodology

["jorgebay (GitHub)" <gi...@apache.org>]

I think it can be simplified into:

```java
traversal.getSteps().stream().filter(step -> step instanceof TraversalParent).forEach(step -> {
    for (final Traversal.Admin<?, ?> local : ((TraversalParent) step).getLocalChildren()) {
        applyTraversalRecursively(consumer, local);
    }
    for (final Traversal.Admin<?, ?> global : ((TraversalParent) step).getGlobalChildren()) {
        applyTraversalRecursively(consumer, global);
    }
});
```

I'm not sure I understand why you were getting concurrent modification exception.

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