-
Notifications
You must be signed in to change notification settings - Fork 861
GValue parity across all GLVs and parameterized feature tests #3456
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a606206
4454ca9
b794045
010f767
c3ccb31
0908959
3487542
80b75ac
835c6c9
0cc4c4b
59881b7
11920b7
44d345a
28fd242
6a6ec06
f7de743
cbab352
6d3f485
78eba56
5d78ae5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -445,7 +445,8 @@ the traversal to utilize a parameter in place of a literal. | |
|
|
||
| [source,go] | ||
| ---- | ||
| g.V().Has("name", gremlingo.NewGValue("name", "marko")) | ||
| g.V().Has("name", gremlingo.GValue{Name: "name", Value: "marko"}) | ||
| g.MergeV(gremlingo.GValue{Name: "vertexPattern", Value: map[interface{}]interface{}{"name": "marko"}}) | ||
| ---- | ||
|
|
||
| [[gremlin-go-scripts]] | ||
|
|
@@ -2212,8 +2213,17 @@ Promise.all([ | |
| [[gremlin-javascript-gvalue]] | ||
| === GValue Parameterization | ||
|
|
||
| IMPORTANT: 4.0.0-beta.2 Release - `GValue` parameterization is not yet implemented for the JavaScript driver. This | ||
| functionality is planned for a future release. | ||
| A `GValue` is an encapsulation of a parameter name and value. A <<traversal-parameterization,subset of gremlin steps>> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
shouldn't we have more docs then? like "GValue Parameterization" sections for all variants?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Javascript was the only GLV which was missing the "GValue Parameterization" section altogether, other than the go GValue construction edit, there were no changes needed to the existing docs here. One slight exception is some GLVs include 2 examples (has() and mergeV()), while others only have the has() example. I'll add the merge example everywhere for consistency. |
||
| are able to accept GValues. When constructing a `GraphTraversal` with such steps in JavaScript, a GValue may be passed | ||
| in the traversal to utilize a parameter in place of a literal. | ||
|
|
||
| [source,javascript] | ||
| ---- | ||
| const GValue = gremlin.process.GValue; | ||
|
|
||
| g.V().has('name', new GValue('name', 'marko')); | ||
| g.mergeV(new GValue('vertexPattern', { name: 'marko' })); | ||
| ---- | ||
|
|
||
| [[gremlin-javascript-scripts]] | ||
| === Submitting Scripts | ||
|
|
@@ -2882,6 +2892,8 @@ the traversal to utilize a parameter in place of a literal. | |
| [source,csharp] | ||
| ---- | ||
| g.V().Has("name", new GValue<string>("name", "marko")); | ||
| g.MergeV(new GValue<IDictionary<object, object>>("vertexPattern", | ||
| new Dictionary<object, object> { { "name", "marko" } })); | ||
| ---- | ||
|
|
||
| [[gremlin-dotnet-scripts]] | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't this PR do a bit more than just this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR did more, but I'm writing the changelog from the perspective of an upgrade from 3.8. This is really the only notable changes from GValue in 3.8 Java. The other GLVs didn't have GValue before as it was tied into bytecode bindings. From the perspective of the 3.8 upgrade, there isn't really changes to GValue in the GLVs, it's a fresh start which is consistent with the existing Java pattern.
I can add a new entry detailing that GValue has been extended to all GLVs, but I don't think the modifications warrant changelog entries here.