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.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
