You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by "spmallette (via GitHub)" <gi...@apache.org> on 2024/02/07 20:31:29 UTC
[PR] TINKERPOP-2862 Added withoutStrategies syntax to grammar [tinkerpop]
spmallette opened a new pull request, #2483:
URL: https://github.com/apache/tinkerpop/pull/2483
https://issues.apache.org/jira/browse/TINKERPOP-2862
Made "new" keyword optional in grammar TINKERPOP-3046. Language variants still have trouble using withoutStrategies() because of TINKERPOP-3047 and TINKERPOP-3048 that will need to get completed before they will work properly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TINKERPOP-2862 Added withoutStrategies syntax to grammar [tinkerpop]
Posted by "vkagamlyk (via GitHub)" <gi...@apache.org>.
vkagamlyk commented on PR #2483:
URL: https://github.com/apache/tinkerpop/pull/2483#issuecomment-1941971681
VOTE +1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TINKERPOP-2862 Added withoutStrategies syntax to grammar [tinkerpop]
Posted by "vkagamlyk (via GitHub)" <gi...@apache.org>.
vkagamlyk commented on code in PR #2483:
URL: https://github.com/apache/tinkerpop/pull/2483#discussion_r1484897917
##########
docs/src/upgrade/release-4.x.x.asciidoc:
##########
@@ -75,10 +75,43 @@ See: link:https://issues.apache.org/jira/browse/TINKERPOP-3017[TINKERPOP-3017]
Starting from this version, `gremlin-javascript` will deserialize `Set` data into a ECMAScript 2015 Set. Previously,
these were deserialized into arrays.
+==== Gremlin Grammar Changes
+
+A number of changes have been introduce to the Gremlin grammar to help make it be more consistent and easier to use.
+
+*`new` keyword is now optional*
+
+The `new` keyword is now optional in all cases where it was previously used. Both of the following examples are now
+valid syntax with the second being the preferred form going forward:
+
+[source,groovy]
+----
+g.V().withStrategies(new SubgraphStrategy(vertices: __.hasLabel('person')))
+
+g.V().withStrategies(SubgraphStrategy(vertices: __.hasLabel('person')))
+----
+
+In a future version, it is likely that the `new` keyword will be removed entirely from the grammar.
+
+*Supports withoutStrategies()*
+
+The `withoutStrategies()` configuration step is now supported syntax for the grammar. While this option is not commonly
+used it is still a part of the Gremlin language and there are times where it is helpful to have this fine grained
+control over how a traversal works.
+
+[source,groovy]
+----
+g.V().withoutStrategies(CountStrategy)
Review Comment:
does it make sense to add an overload that accepts string as argument? Might be useful for GLV's, like
`withoutStrategies(SubgraphStrategy.class)` vs `withoutStrategies("SubgraphStrategy")`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TINKERPOP-2862 Added withoutStrategies syntax to grammar [tinkerpop]
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette merged PR #2483:
URL: https://github.com/apache/tinkerpop/pull/2483
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TINKERPOP-2862 Added withoutStrategies syntax to grammar [tinkerpop]
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #2483:
URL: https://github.com/apache/tinkerpop/pull/2483#discussion_r1487827851
##########
docs/src/upgrade/release-4.x.x.asciidoc:
##########
@@ -75,10 +75,43 @@ See: link:https://issues.apache.org/jira/browse/TINKERPOP-3017[TINKERPOP-3017]
Starting from this version, `gremlin-javascript` will deserialize `Set` data into a ECMAScript 2015 Set. Previously,
these were deserialized into arrays.
+==== Gremlin Grammar Changes
+
+A number of changes have been introduce to the Gremlin grammar to help make it be more consistent and easier to use.
+
+*`new` keyword is now optional*
+
+The `new` keyword is now optional in all cases where it was previously used. Both of the following examples are now
+valid syntax with the second being the preferred form going forward:
+
+[source,groovy]
+----
+g.V().withStrategies(new SubgraphStrategy(vertices: __.hasLabel('person')))
+
+g.V().withStrategies(SubgraphStrategy(vertices: __.hasLabel('person')))
+----
+
+In a future version, it is likely that the `new` keyword will be removed entirely from the grammar.
+
+*Supports withoutStrategies()*
+
+The `withoutStrategies()` configuration step is now supported syntax for the grammar. While this option is not commonly
+used it is still a part of the Gremlin language and there are times where it is helpful to have this fine grained
+control over how a traversal works.
+
+[source,groovy]
+----
+g.V().withoutStrategies(CountStrategy)
Review Comment:
It doesn't need a String. The grammar should accept any `Identifier` there. If it matches a registered strategy from a provider, it will work as valid syntax.
https://github.com/apache/tinkerpop/pull/2483/files#diff-39ed59720093773649512a14aeba8c3da9e6d21ce016c7f0cdf7a1242e3a76a1R72
I suppose it will be up to each programming language to decide what to do with that. For Java, you could have reference to the actual class file from the provider and could use that. Since it matches on the simple name of the class a dummy class could also be used. Alternatively, Java (and other languages) could be changed to allow a `String` I suppose, but I wasn't ready to make that choice. I mostly wanted to focus in on the grammar and getting that working right for standard TinkerPop strategies. Created: https://issues.apache.org/jira/browse/TINKERPOP-3055 to track.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TINKERPOP-2862 Added withoutStrategies syntax to grammar [tinkerpop]
Posted by "vkagamlyk (via GitHub)" <gi...@apache.org>.
vkagamlyk commented on code in PR #2483:
URL: https://github.com/apache/tinkerpop/pull/2483#discussion_r1484897917
##########
docs/src/upgrade/release-4.x.x.asciidoc:
##########
@@ -75,10 +75,43 @@ See: link:https://issues.apache.org/jira/browse/TINKERPOP-3017[TINKERPOP-3017]
Starting from this version, `gremlin-javascript` will deserialize `Set` data into a ECMAScript 2015 Set. Previously,
these were deserialized into arrays.
+==== Gremlin Grammar Changes
+
+A number of changes have been introduce to the Gremlin grammar to help make it be more consistent and easier to use.
+
+*`new` keyword is now optional*
+
+The `new` keyword is now optional in all cases where it was previously used. Both of the following examples are now
+valid syntax with the second being the preferred form going forward:
+
+[source,groovy]
+----
+g.V().withStrategies(new SubgraphStrategy(vertices: __.hasLabel('person')))
+
+g.V().withStrategies(SubgraphStrategy(vertices: __.hasLabel('person')))
+----
+
+In a future version, it is likely that the `new` keyword will be removed entirely from the grammar.
+
+*Supports withoutStrategies()*
+
+The `withoutStrategies()` configuration step is now supported syntax for the grammar. While this option is not commonly
+used it is still a part of the Gremlin language and there are times where it is helpful to have this fine grained
+control over how a traversal works.
+
+[source,groovy]
+----
+g.V().withoutStrategies(CountStrategy)
Review Comment:
does it make sense to add an overload that accepts string as argument? Might be useful for GLV's, like
`withoutStrategies(SubgraphStrategy.class)` vs `withoutStrategies("SubgraphStrategy")`. Also for providers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TINKERPOP-2862 Added withoutStrategies syntax to grammar [tinkerpop]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2483:
URL: https://github.com/apache/tinkerpop/pull/2483#issuecomment-1932820461
## [Codecov](https://app.codecov.io/gh/apache/tinkerpop/pull/2483?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`fae51e8`)](https://app.codecov.io/gh/apache/tinkerpop/commit/fae51e87e7362648e53e58c66206a3a0442caa99?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 76.16% compared to head [(`5a30307`)](https://app.codecov.io/gh/apache/tinkerpop/pull/2483?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 71.08%.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #2483 +/- ##
============================================
- Coverage 76.16% 71.08% -5.09%
============================================
Files 1085 25 -1060
Lines 65191 3949 -61242
Branches 7289 0 -7289
============================================
- Hits 49654 2807 -46847
+ Misses 12836 951 -11885
+ Partials 2701 191 -2510
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/tinkerpop/pull/2483?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TINKERPOP-2862 Added withoutStrategies syntax to grammar [tinkerpop]
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on PR #2483:
URL: https://github.com/apache/tinkerpop/pull/2483#issuecomment-1942128603
LGTM VOTE +1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TINKERPOP-2862 Added withoutStrategies syntax to grammar [tinkerpop]
Posted by "vkagamlyk (via GitHub)" <gi...@apache.org>.
vkagamlyk commented on code in PR #2483:
URL: https://github.com/apache/tinkerpop/pull/2483#discussion_r1484898149
##########
docs/src/upgrade/release-4.x.x.asciidoc:
##########
@@ -75,10 +75,43 @@ See: link:https://issues.apache.org/jira/browse/TINKERPOP-3017[TINKERPOP-3017]
Starting from this version, `gremlin-javascript` will deserialize `Set` data into a ECMAScript 2015 Set. Previously,
these were deserialized into arrays.
+==== Gremlin Grammar Changes
+
+A number of changes have been introduce to the Gremlin grammar to help make it be more consistent and easier to use.
+
+*`new` keyword is now optional*
+
+The `new` keyword is now optional in all cases where it was previously used. Both of the following examples are now
+valid syntax with the second being the preferred form going forward:
+
+[source,groovy]
+----
+g.V().withStrategies(new SubgraphStrategy(vertices: __.hasLabel('person')))
+
+g.V().withStrategies(SubgraphStrategy(vertices: __.hasLabel('person')))
+----
+
+In a future version, it is likely that the `new` keyword will be removed entirely from the grammar.
+
+*Supports withoutStrategies()*
+
+The `withoutStrategies()` configuration step is now supported syntax for the grammar. While this option is not commonly
+used it is still a part of the Gremlin language and there are times where it is helpful to have this fine grained
+control over how a traversal works.
+
+[source,groovy]
+----
+g.V().withoutStrategies(CountStrategy)
Review Comment:
also for providers
##########
docs/src/upgrade/release-4.x.x.asciidoc:
##########
@@ -75,10 +75,43 @@ See: link:https://issues.apache.org/jira/browse/TINKERPOP-3017[TINKERPOP-3017]
Starting from this version, `gremlin-javascript` will deserialize `Set` data into a ECMAScript 2015 Set. Previously,
these were deserialized into arrays.
+==== Gremlin Grammar Changes
+
+A number of changes have been introduce to the Gremlin grammar to help make it be more consistent and easier to use.
+
+*`new` keyword is now optional*
+
+The `new` keyword is now optional in all cases where it was previously used. Both of the following examples are now
+valid syntax with the second being the preferred form going forward:
+
+[source,groovy]
+----
+g.V().withStrategies(new SubgraphStrategy(vertices: __.hasLabel('person')))
+
+g.V().withStrategies(SubgraphStrategy(vertices: __.hasLabel('person')))
+----
+
+In a future version, it is likely that the `new` keyword will be removed entirely from the grammar.
+
+*Supports withoutStrategies()*
+
+The `withoutStrategies()` configuration step is now supported syntax for the grammar. While this option is not commonly
+used it is still a part of the Gremlin language and there are times where it is helpful to have this fine grained
+control over how a traversal works.
+
+[source,groovy]
+----
+g.V().withoutStrategies(CountStrategy)
Review Comment:
also for providers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org