-
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
base: master
Are you sure you want to change the base?
Changes from all commits
0bccf87
c2e2816
e7b4789
aa766f2
3f2af70
6ca6d4d
0744888
35bef9c
c9dba53
d67f95b
96939ca
836718a
25cc2cd
19f7301
25d14ed
6d2e1c1
d0e3fad
4c58e17
1819657
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 |
|---|---|---|
|
|
@@ -337,7 +337,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]] | ||
|
|
@@ -1852,8 +1853,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 | ||
|
|
@@ -2398,6 +2408,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.