Skip to content

TINKERPOP-2075 Added ReferenceElementStrategy#1010

Merged
spmallette merged 1 commit intomasterfrom
TINKERPOP-2075
Dec 10, 2018
Merged

TINKERPOP-2075 Added ReferenceElementStrategy#1010
spmallette merged 1 commit intomasterfrom
TINKERPOP-2075

Conversation

@spmallette
Copy link
Contributor

@spmallette spmallette commented Dec 3, 2018

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

This strategy converts all graph elements to references. Configured this strategy in Gremlin Server by default for the example graphs. Made a few minor updates to tests that were assuming the return of "detached" elements.

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

VOTE +1

This strategy converts all graph elements to references. Configured this strategy in Gremlin Server by default for the example graphs. Made a few minor updates to tests that were assuming the return of "detached" elements.
@dkuppitz
Copy link
Contributor

dkuppitz commented Dec 3, 2018

Do you have a scenario in mind where providers would construct a new ReferenceElement step? I don't think it's necessary and I guess if you would have thought of such a scenario, you wouldn't have nested the class inside ReferenceElementStrategy; hence I think ReferenceElement constructor can be package-private.

Anyway, since this is just a minor thing ...

VOTE +1

@spmallette
Copy link
Contributor Author

yeah.........i'll sleep on that. wasn't sure what to do with it. such a dumb little step.

oh, i wanted to ask....this should be a finalization strategy right? i tried it as a decoration at first and it didn't work right, so i figured it needed to be applied at the end and that seemed to fix things. 🤷‍♂️ should i have done something else?

@dkuppitz
Copy link
Contributor

dkuppitz commented Dec 4, 2018

No, FinalizationStrategy is correct, nothing else to do here.

@spmallette spmallette merged commit 2cb95c9 into master Dec 10, 2018
@spmallette spmallette deleted the TINKERPOP-2075 branch December 10, 2018 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants