[ 
https://issues.apache.org/jira/browse/TINKERPOP-3055?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17947097#comment-17947097
 ] 

ASF GitHub Bot commented on TINKERPOP-3055:
-------------------------------------------

andreachild commented on code in PR #3099:
URL: https://github.com/apache/tinkerpop/pull/3099#discussion_r2058910838


##########
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/traversal-strategy.js:
##########
@@ -239,6 +276,16 @@ class PathRetractionStrategy extends TraversalStrategy {
   }
 }
 
+class ProductiveByStrategy extends TraversalStrategy {
+  /**
+   * @param {Object} [options]
+   * @param {Array<String>} [options.productiveKeys] set of keys that will 
always be productive
+   */
+  constructor(options) {
+    
super('org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.ProductiveByStrategy',
 options);

Review Comment:
   I'm curious why in this case the full `options` are passed through to the 
super constructor but for other strategies like `SeedStrategy` only specific 
options are passed through?





> withoutStrategies() mechanism in programming languages for providers
> --------------------------------------------------------------------
>
>                 Key: TINKERPOP-3055
>                 URL: https://issues.apache.org/jira/browse/TINKERPOP-3055
>             Project: TinkerPop
>          Issue Type: Improvement
>          Components: dotnet, go, javascript, process, python
>    Affects Versions: 3.7.1
>            Reporter: Stephen Mallette
>            Priority: Major
>
> {{withoutStrategies()}} is in the grammar for TINKERPOP-2862. That change did 
> not address its accessibility for provider strategies in language variants 
> very well. As the syntax requires a {{Class}} (and for the grammar, a 
> registered strategy class) you may not have that reference in a language 
> variant. Users could create dummy classes as the grammar works on simple 
> name, but that's not especially nice. Otoh, most users shouldn't be tinkering 
> with strategies so perhaps that's ok? It could be inconvenient for notebook 
> users and similar tools though to create the dummy. A simple alternative 
> could just be a {{withoutStrategies(String...)}} but that's not particularly 
> nice. Other ideas?
> needs a general look at all strategy construction across all languages:
> 1. check if the strategy construction makes sense in terms of types and 
> syntax in each language
> 2. watch out for wrong types being parsed into {{Configuration}} which can 
> lead to weird looking errors. 
> 3. are there adequate tests to validate all our syntax is working. we 
> technically need to test every strategy configuration options as those 
> corners are where bugs can hide.
> 4. double check {{List}} vs {{Set}} syntax because {{Set}} might be preferred 
> but a lot of folks will reach for {{[ ]}} just out of habit....do we want 
> them failing for that? can we ease the type there without losing {{Set}} in 
> type safe languages?



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to