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 <gi...@git.apache.org> on 2018/03/22 19:19:05 UTC
[GitHub] tinkerpop pull request #823: TINKERPOP-1866 Support g:T for .NET
GitHub user spmallette opened a pull request:
https://github.com/apache/tinkerpop/pull/823
TINKERPOP-1866 Support g:T for .NET
https://issues.apache.org/jira/browse/TINKERPOP-1866
Issuing this PR for @FlorianHockmann now that TINKERPOP-1865 has merged.
Builds with ` mvn clean install -DskipTests && mvn verify -pl :gremlin-dotnet-tests -DskipIntegrationTests=false`
VOTE +1
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/tinkerpop TINKERPOP-1866
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/823.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #823
----
commit f88cb88a8622222e3a8911e5f502af31297a41bc
Author: Florian Hockmann <fh...@...>
Date: 2018-03-18T16:19:45Z
TINKERPOP-1865 Run Gremlin.Net tests with GraphSON 3.0
This required the rewrite of the GetEdges() function for the Gherkin
test runner. Additionally, 3 tests had to be ignored: 1 has the
T deserialization problem and 2 get data returned from the server that
doesn't match the expected data.
commit b9c160244d6570c457a4ae52bdab1aa80e388c4c
Author: Florian Hockmann <fh...@...>
Date: 2018-03-20T14:45:11Z
TINKERPOP-1866 Add a deserializer for g:T to Gremlin.Net
----
---
[GitHub] tinkerpop pull request #823: TINKERPOP-1866 Support g:T for .NET
Posted by dkuppitz <gi...@git.apache.org>.
Github user dkuppitz commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/823#discussion_r176565903
--- Diff: gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/ScenarioData.cs ---
@@ -30,6 +30,7 @@
using Gremlin.Net.Process.Remote;
using Gremlin.Net.Process.Traversal;
using Gremlin.Net.Structure;
+using static Gremlin.Net.Process.Traversal.__;
--- End diff --
Seems like an accident, as it wasn't needed before and nothing else has changed in this file.
---
[GitHub] tinkerpop pull request #823: TINKERPOP-1866 Support g:T for .NET
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/823#discussion_r176566595
--- Diff: gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/ScenarioData.cs ---
@@ -30,6 +30,7 @@
using Gremlin.Net.Process.Remote;
using Gremlin.Net.Process.Traversal;
using Gremlin.Net.Structure;
+using static Gremlin.Net.Process.Traversal.__;
--- End diff --
i saw that, but didn't kill it b/c it was Florian's changes. i guess i should have questioned it. i will try to remove that change locally and see what happens.
---
[GitHub] tinkerpop pull request #823: TINKERPOP-1866 Support g:T for .NET
Posted by dkuppitz <gi...@git.apache.org>.
Github user dkuppitz commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/823#discussion_r176568322
--- Diff: gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/CommonSteps.cs ---
@@ -239,7 +240,8 @@ private static object ToLambda(string stringLambda, string graphName)
private static object ToT(string enumName, string graphName)
--- End diff --
No, not an issue, I was just curious.
---
[GitHub] tinkerpop pull request #823: TINKERPOP-1866 Support g:T for .NET
Posted by dkuppitz <gi...@git.apache.org>.
Github user dkuppitz commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/823#discussion_r176565685
--- Diff: gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/CommonSteps.cs ---
@@ -239,7 +240,8 @@ private static object ToLambda(string stringLambda, string graphName)
private static object ToT(string enumName, string graphName)
--- End diff --
What's the point of the `graphName` argument?
---
[GitHub] tinkerpop pull request #823: TINKERPOP-1866 Support g:T for .NET
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/823#discussion_r176569502
--- Diff: gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/ScenarioData.cs ---
@@ -30,6 +30,7 @@
using Gremlin.Net.Process.Remote;
using Gremlin.Net.Process.Traversal;
using Gremlin.Net.Structure;
+using static Gremlin.Net.Process.Traversal.__;
--- End diff --
that commit was irrelevant - killed it out and force pushed
---
[GitHub] tinkerpop pull request #823: TINKERPOP-1866 Support g:T for .NET
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/tinkerpop/pull/823
---
[GitHub] tinkerpop issue #823: TINKERPOP-1866 Support g:T for .NET
Posted by dkuppitz <gi...@git.apache.org>.
Github user dkuppitz commented on the issue:
https://github.com/apache/tinkerpop/pull/823
VOTE: +1
---
[GitHub] tinkerpop pull request #823: TINKERPOP-1866 Support g:T for .NET
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/823#discussion_r176684867
--- Diff: gremlin-dotnet/src/Gremlin.Net/Process/Traversal/EnumWrapper.cs ---
@@ -48,5 +50,32 @@ protected EnumWrapper(string enumName, string enumValue)
EnumName = enumName;
EnumValue = enumValue;
}
+
+ /// <inheritdoc />
+ public bool Equals(EnumWrapper other)
--- End diff --
Nice! we've totally missed equality checks as part of 1901.
---
[GitHub] tinkerpop pull request #823: TINKERPOP-1866 Support g:T for .NET
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/823#discussion_r176567525
--- Diff: gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/CommonSteps.cs ---
@@ -239,7 +240,8 @@ private static object ToLambda(string stringLambda, string graphName)
private static object ToT(string enumName, string graphName)
--- End diff --
It's not really useful to this conversion, but it's the expected signature for the lambda where this method is used. See further up in the definition of `Parsers` where it requires a `Func<string,string,object>` a two arg function basically. maybe we could make that nicer, but this is all plumbing code for tests, so perhaps not a big issue?
---