[
https://issues.apache.org/jira/browse/TINKERPOP-3247?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18078775#comment-18078775
]
ASF GitHub Bot commented on TINKERPOP-3247:
-------------------------------------------
Cole-Greer commented on code in PR #3402:
URL: https://github.com/apache/tinkerpop/pull/3402#discussion_r3190325731
##########
gremlin-js/gremlin-javascript/lib/driver/request-message.ts:
##########
@@ -128,16 +131,24 @@ export class Builder {
}
addBinding(key: string, value: any): Builder {
+ if (this.bindingsString) throw new Error('Cannot mix addBinding() with
addBindingsString().');
Object.assign(this.bindings, {[key]: value})
return this;
}
addBindings(otherBindings: object): Builder {
if (!otherBindings) throw new Error('bindings argument cannot be null.');
+ if (this.bindingsString) throw new Error('Cannot mix addBindings() with
addBindingsString().');
Object.assign(this.bindings, otherBindings)
return this;
}
+ addBindingsString(bindingsString: string): Builder {
Review Comment:
Nit: `addBindings` acts as a merge, but `addBindingsString` is a
replacement. I think that's ok to an extent, (we definitely don't want to try
merging strings), but might be worth a quick jsdoc. Perhaps it would be also
reasonable to only allow `addBindingsString` to be called once.
> String-Based Parameters
> -----------------------
>
> Key: TINKERPOP-3247
> URL: https://issues.apache.org/jira/browse/TINKERPOP-3247
> Project: TinkerPop
> Issue Type: Improvement
> Components: language, server
> Affects Versions: 4.0.0
> Reporter: Ken Hu
> Priority: Major
>
> Based on the discussion in the devlist, this revolves changing the
> bindings/parameters from being a Map to a gremlin-lang string version of the
> map. This decouples the evolution of the language from the evolution of the
> serializers as new types can be added without having to update the
> serializers as well.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
