Re: [PR] feat: TypeScript integration [tinkerpop]
vkagamlyk merged PR #2515: URL: https://github.com/apache/tinkerpop/pull/2515 -- 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-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
vkagamlyk commented on PR #2515: URL: https://github.com/apache/tinkerpop/pull/2515#issuecomment-2004578580 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-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
vkagamlyk commented on PR #2515: URL: https://github.com/apache/tinkerpop/pull/2515#issuecomment-1998346909 I think the code is good and would like to see it merged. But this is a very big change that may be important to other people, so I would recommend writing a short message to [dev list](https://tinkerpop.apache.org/docs/current/dev/developer/#ways-to-contribute), describing the changes and asking if there are any objections. Feel free to reach me out in Discord if you have any question or need help with this. -- 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-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
tien commented on PR #2515:
URL: https://github.com/apache/tinkerpop/pull/2515#issuecomment-1993622235
Have updated the browser example to use TypeScript instead. Didn't touch the
node example as it's right now just a collection of scripts :p
NOTE:
You can see from the browser example that importing type definition is not
the best right now in terms of ergonomic. Addressing this will require changing
the export a little, which I think should be done as a separate PR.
```ts
import type { Edge, Vertex } from "gremlin/build/esm/structure/graph";
```
--
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-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
tien commented on code in PR #2515:
URL: https://github.com/apache/tinkerpop/pull/2515#discussion_r1522535064
##
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/graph.ts:
##
@@ -46,43 +44,49 @@ class Graph {
}
class Element {
- constructor(id, label) {
-this.id = id;
-this.label = label;
- }
+ constructor(
+readonly id: string,
+readonly label: string,
+ ) {}
/**
* Compares this instance to another and determines if they can be
considered as equal.
* @param {Element} other
* @returns {boolean}
*/
- equals(other) {
+ equals(other: Element) {
Review Comment:
Yes, this make sense, I've updated accordingly.
--
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-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
tien commented on code in PR #2515:
URL: https://github.com/apache/tinkerpop/pull/2515#discussion_r1522534690
##
gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/feature-steps.js:
##
@@ -20,28 +20,28 @@
/**
* @author Jorge Bay Gondra
*/
-'use strict';
-const {Given, Then, When, setDefaultTimeout} = require('cucumber');
+import chaiString from 'chai-string';
+import { Given, Then, When, setDefaultTimeout } from '@cucumber/cucumber';
// Setting Cucumber timeout to 10s for Floating Errors on Windows on GitHub
Actions
setDefaultTimeout(10 * 1000);
-const chai = require('chai')
-chai.use(require('chai-string'));
-const expect = chai.expect;
-const util = require('util');
-const gremlin = require('./gremlin').gremlin;
-const graphModule = require('../../lib/structure/graph');
-const graphTraversalModule = require('../../lib/process/graph-traversal');
-const traversalModule = require('../../lib/process/traversal');
-const utils = require('../../lib/utils');
-const traversal = require('../../lib/process/anonymous-traversal').traversal;
-const Path = graphModule.Path;
-const __ = graphTraversalModule.statics;
-const t = traversalModule.t;
-const P = traversalModule.P;
-const direction = traversalModule.direction;
-const merge = traversalModule.merge;
-const deepMembersById = require('./element-comparison').deepMembersById;
+import { use, expect as _expect } from 'chai';
+use(chaiString);
+const expect = _expect;
+import { inspect, format, inherits } from 'util';
+import { gremlin } from './gremlin.js';
+import { Path as _Path, Vertex, Edge } from '../../lib/structure/graph.js';
Review Comment:
Yep, fixed all the weird auto refactor issues.
--
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-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
vkagamlyk commented on code in PR #2515:
URL: https://github.com/apache/tinkerpop/pull/2515#discussion_r1522533103
##
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/graph.ts:
##
@@ -46,43 +44,49 @@ class Graph {
}
class Element {
- constructor(id, label) {
-this.id = id;
-this.label = label;
- }
+ constructor(
+readonly id: string,
+readonly label: string,
+ ) {}
/**
* Compares this instance to another and determines if they can be
considered as equal.
* @param {Element} other
* @returns {boolean}
*/
- equals(other) {
+ equals(other: Element) {
Review Comment:
same for `Path` and `Property`
--
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-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
tien commented on code in PR #2515:
URL: https://github.com/apache/tinkerpop/pull/2515#discussion_r1522528198
##
gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/feature-steps.js:
##
@@ -20,28 +20,28 @@
/**
* @author Jorge Bay Gondra
*/
-'use strict';
-const {Given, Then, When, setDefaultTimeout} = require('cucumber');
+import chaiString from 'chai-string';
+import { Given, Then, When, setDefaultTimeout } from '@cucumber/cucumber';
// Setting Cucumber timeout to 10s for Floating Errors on Windows on GitHub
Actions
setDefaultTimeout(10 * 1000);
-const chai = require('chai')
-chai.use(require('chai-string'));
-const expect = chai.expect;
-const util = require('util');
-const gremlin = require('./gremlin').gremlin;
-const graphModule = require('../../lib/structure/graph');
-const graphTraversalModule = require('../../lib/process/graph-traversal');
-const traversalModule = require('../../lib/process/traversal');
-const utils = require('../../lib/utils');
-const traversal = require('../../lib/process/anonymous-traversal').traversal;
-const Path = graphModule.Path;
-const __ = graphTraversalModule.statics;
-const t = traversalModule.t;
-const P = traversalModule.P;
-const direction = traversalModule.direction;
-const merge = traversalModule.merge;
-const deepMembersById = require('./element-comparison').deepMembersById;
+import { use, expect as _expect } from 'chai';
+use(chaiString);
+const expect = _expect;
+import { inspect, format, inherits } from 'util';
+import { gremlin } from './gremlin.js';
+import { Path as _Path, Vertex, Edge } from '../../lib/structure/graph.js';
Review Comment:
Hmm, I used the refactor functionality of VSCode to change these, look like
it did something funny with default export. Let me do a quick scan n fix all
these.
--
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-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
vkagamlyk commented on code in PR #2515:
URL: https://github.com/apache/tinkerpop/pull/2515#discussion_r1522524987
##
gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/feature-steps.js:
##
@@ -20,28 +20,28 @@
/**
* @author Jorge Bay Gondra
*/
-'use strict';
-const {Given, Then, When, setDefaultTimeout} = require('cucumber');
+import chaiString from 'chai-string';
+import { Given, Then, When, setDefaultTimeout } from '@cucumber/cucumber';
// Setting Cucumber timeout to 10s for Floating Errors on Windows on GitHub
Actions
setDefaultTimeout(10 * 1000);
-const chai = require('chai')
-chai.use(require('chai-string'));
-const expect = chai.expect;
-const util = require('util');
-const gremlin = require('./gremlin').gremlin;
-const graphModule = require('../../lib/structure/graph');
-const graphTraversalModule = require('../../lib/process/graph-traversal');
-const traversalModule = require('../../lib/process/traversal');
-const utils = require('../../lib/utils');
-const traversal = require('../../lib/process/anonymous-traversal').traversal;
-const Path = graphModule.Path;
-const __ = graphTraversalModule.statics;
-const t = traversalModule.t;
-const P = traversalModule.P;
-const direction = traversalModule.direction;
-const merge = traversalModule.merge;
-const deepMembersById = require('./element-comparison').deepMembersById;
+import { use, expect as _expect } from 'chai';
+use(chaiString);
+const expect = _expect;
+import { inspect, format, inherits } from 'util';
+import { gremlin } from './gremlin.js';
+import { Path as _Path, Vertex, Edge } from '../../lib/structure/graph.js';
Review Comment:
Why not just
`import { Path, Vertex, Edge } from '../../lib/structure/graph.js';` ?
--
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-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
tien commented on code in PR #2515:
URL: https://github.com/apache/tinkerpop/pull/2515#discussion_r1522509577
##
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/graph.ts:
##
@@ -46,43 +44,49 @@ class Graph {
}
class Element {
- constructor(id, label) {
-this.id = id;
-this.label = label;
- }
+ constructor(
+readonly id: string,
+readonly label: string,
+ ) {}
/**
* Compares this instance to another and determines if they can be
considered as equal.
* @param {Element} other
* @returns {boolean}
*/
- equals(other) {
+ equals(other: Element) {
Review Comment:
Thanks, I've updated the type here to `any`.
--
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-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
tien commented on code in PR #2515:
URL: https://github.com/apache/tinkerpop/pull/2515#discussion_r1522509374
##
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/traversal.ts:
##
@@ -476,49 +481,43 @@ function toEnum(typeName, keys) {
const directionAlias = {
from_: 'out',
to: 'in',
-};
+} as const;
// for direction enums, maps the same EnumValue object to the enum aliases
after creating them
-function toDirectionEnum(typeName, keys) {
+function toDirectionEnum(typeName: string, keys: string) {
const result = toEnum(typeName, keys);
Object.keys(directionAlias).forEach((k) => {
-result[k] = result[directionAlias[k]];
+result[k] = result[directionAlias[k as keyof typeof directionAlias]];
});
return result;
}
-class EnumValue {
- constructor(typeName, elementName) {
-this.typeName = typeName;
-this.elementName = elementName;
- }
+export class EnumValue {
+ constructor(
+public typeName: string,
+public elementName: string,
+ ) {}
toString() {
return this.elementName;
}
}
-module.exports = {
- EnumValue,
- P,
- TextP,
- withOptions,
- IO,
- Traversal,
- TraversalSideEffects,
- Traverser,
- barrier: toEnum('Barrier', 'normSack'),
- cardinality: toEnum('Cardinality', 'list set single'),
- column: toEnum('Column', 'keys values'),
- direction: toDirectionEnum('Direction', 'BOTH IN OUT from_ to'),
- dt: toEnum('DT', 'second minute hour day'),
- graphSONVersion: toEnum('GraphSONVersion', 'V1_0 V2_0 V3_0'),
- gryoVersion: toEnum('GryoVersion', 'V1_0 V3_0'),
- merge: toEnum('Merge', 'onCreate onMatch outV inV'),
- operator: toEnum('Operator', 'addAll and assign div max min minus mult or
sum sumLong'),
- order: toEnum('Order', 'asc desc shuffle'),
- pick: toEnum('Pick', 'any none'),
- pop: toEnum('Pop', 'all first last mixed'),
- scope: toEnum('Scope', 'global local'),
- t: toEnum('T', 'id key label value'),
-};
+// export const barrier = toEnum('Barrier', 'normSack');
Review Comment:
Oops, have removed these
--
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-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
vkagamlyk commented on code in PR #2515:
URL: https://github.com/apache/tinkerpop/pull/2515#discussion_r1522501485
##
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/graph.ts:
##
@@ -46,43 +44,49 @@ class Graph {
}
class Element {
- constructor(id, label) {
-this.id = id;
-this.label = label;
- }
+ constructor(
+readonly id: string,
+readonly label: string,
+ ) {}
/**
* Compares this instance to another and determines if they can be
considered as equal.
* @param {Element} other
* @returns {boolean}
*/
- equals(other) {
+ equals(other: Element) {
Review Comment:
other can by `any`
--
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-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
vkagamlyk commented on code in PR #2515:
URL: https://github.com/apache/tinkerpop/pull/2515#discussion_r1522500544
##
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/traversal.ts:
##
@@ -476,49 +481,43 @@ function toEnum(typeName, keys) {
const directionAlias = {
from_: 'out',
to: 'in',
-};
+} as const;
// for direction enums, maps the same EnumValue object to the enum aliases
after creating them
-function toDirectionEnum(typeName, keys) {
+function toDirectionEnum(typeName: string, keys: string) {
const result = toEnum(typeName, keys);
Object.keys(directionAlias).forEach((k) => {
-result[k] = result[directionAlias[k]];
+result[k] = result[directionAlias[k as keyof typeof directionAlias]];
});
return result;
}
-class EnumValue {
- constructor(typeName, elementName) {
-this.typeName = typeName;
-this.elementName = elementName;
- }
+export class EnumValue {
+ constructor(
+public typeName: string,
+public elementName: string,
+ ) {}
toString() {
return this.elementName;
}
}
-module.exports = {
- EnumValue,
- P,
- TextP,
- withOptions,
- IO,
- Traversal,
- TraversalSideEffects,
- Traverser,
- barrier: toEnum('Barrier', 'normSack'),
- cardinality: toEnum('Cardinality', 'list set single'),
- column: toEnum('Column', 'keys values'),
- direction: toDirectionEnum('Direction', 'BOTH IN OUT from_ to'),
- dt: toEnum('DT', 'second minute hour day'),
- graphSONVersion: toEnum('GraphSONVersion', 'V1_0 V2_0 V3_0'),
- gryoVersion: toEnum('GryoVersion', 'V1_0 V3_0'),
- merge: toEnum('Merge', 'onCreate onMatch outV inV'),
- operator: toEnum('Operator', 'addAll and assign div max min minus mult or
sum sumLong'),
- order: toEnum('Order', 'asc desc shuffle'),
- pick: toEnum('Pick', 'any none'),
- pop: toEnum('Pop', 'all first last mixed'),
- scope: toEnum('Scope', 'global local'),
- t: toEnum('T', 'id key label value'),
-};
+// export const barrier = toEnum('Barrier', 'normSack');
Review Comment:
nit: looks like duplicated code
--
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-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
tien commented on PR #2515: URL: https://github.com/apache/tinkerpop/pull/2515#issuecomment-1993445516 Added a small entry to the `changelog` file, please let me know if further info or modification is needed. -- 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-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
tien commented on PR #2515: URL: https://github.com/apache/tinkerpop/pull/2515#issuecomment-1992675130 @vkagamlyk I've removed all the unnecessary changes to the test files. Also reset them all to what is on `master` first so there are no conflicts from `3.7`. -- 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-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
tien commented on PR #2515: URL: https://github.com/apache/tinkerpop/pull/2515#issuecomment-1992506652 > documentation update for [users](https://tinkerpop.apache.org/docs/current/reference/#gremlin-javascript), [developers environment](https://tinkerpop.apache.org/docs/current/dev/developer/#nodejs-environment) and [release process](https://tinkerpop.apache.org/docs/current/dev/developer/#_release_process). Maybe I missed somewhere else. Maybe this part is not needed, since I've just make this change completely backward compatible: - User documentation: still should update the user usage documentation to use newer ES module import syntax though, but maybe in a separate PR to reduce the number of changes - Developer environment: this should stay the exact same - Release process: I think this should also be the exact same also, because the build step should be automatically executed every time you run `npm publish`. You can test this right now by doing `npm publish --dry-run` to see what will get published. -- 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-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
tien commented on PR #2515: URL: https://github.com/apache/tinkerpop/pull/2515#issuecomment-1992495561 > P.S. Please do not change the formatting because it makes code review very difficult. It's usually better to make separate PR for formatting/fix styles. Yeah, this is because my editor format code on save by default if the project has a prettier config. Will spend a bit of time later today to undo all the formatting changes. -- 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-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
tien commented on code in PR #2515:
URL: https://github.com/apache/tinkerpop/pull/2515#discussion_r1522052097
##
gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/sasl-authentication-tests.js:
##
@@ -69,38 +68,43 @@ describe('DriverRemoteConnection', function () {
it('should return error when using ws:// for a TLS configured server',
function () {
const authenticator = new PlainTextSaslAuthenticator('stephen',
'password');
-connection = helper.getDriverRemoteConnection(badServerAuthUrl, {
+connection = getDriverRemoteConnection(badServerAuthUrl, {
authenticator: authenticator,
- rejectUnauthorized: false
+ rejectUnauthorized: false,
});
-return connection.submit(new Bytecode().addStep('V',
[]).addStep('tail', []))
-.then(function() {
- assert.fail("server is running TLS and trying to connect with
ws:// so this should result in error thrown");
-})
-.catch(function (err) {
- if (err instanceof AssertionError) throw err;
- assert.ok(err);
- assert.ok(err.message === 'socket hang up');
-});
+return connection
+ .submit(new Bytecode().addStep('V', []).addStep('tail', []))
+ .then(function () {
+assert.fail('server is running TLS and trying to connect with
ws:// so this should result in error thrown');
+ })
+ .catch(function (err) {
+if (err instanceof AssertionError) throw err;
+assert.ok(err);
+assert.ok(err.message === 'socket hang up');
+ });
});
it('should return error when using ws:// for a TLS configured server',
function () {
const authenticator = new PlainTextSaslAuthenticator('stephen',
'password');
-connection = helper.getDriverRemoteConnection(badServerAuthUrl, {
+connection = getDriverRemoteConnection(badServerAuthUrl, {
authenticator: authenticator,
- rejectUnauthorized: false
+ rejectUnauthorized: false,
});
-const g = traversal().with_(connection);
-return g.V().toList().then(function() {
- assert.fail("server is running TLS and trying to connect with ws://
so this should result in error thrown");
-}).catch(function(err) {
-if (err instanceof AssertionError) throw err;
- assert.ok(err);
- assert.ok(err.message === 'socket hang up');
+const g = traversal().withRemote(connection);
Review Comment:
Oops, must have been brought over since I originally did this on the 3.7
branch. Have replaced all `withRemote` with `with_`
--
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-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
vkagamlyk commented on PR #2515: URL: https://github.com/apache/tinkerpop/pull/2515#issuecomment-1992488713 Missing parts: - documentation update for [users](https://tinkerpop.apache.org/docs/current/reference/#gremlin-javascript), [developers environment](https://tinkerpop.apache.org/docs/current/dev/developer/#nodejs-environment) and [release process](https://tinkerpop.apache.org/docs/current/dev/developer/#_release_process). Maybe I missed somewhere else. - update nodejs examples if needed. - update `changelog` file. Also good to double check if packaging works with new changes. P.S. Please do not change the formatting because it makes code review very difficult. It's usually better to make separate PR for formatting/fix styles. -- 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-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
vkagamlyk commented on code in PR #2515:
URL: https://github.com/apache/tinkerpop/pull/2515#discussion_r1522028418
##
gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/sasl-authentication-tests.js:
##
@@ -69,38 +68,43 @@ describe('DriverRemoteConnection', function () {
it('should return error when using ws:// for a TLS configured server',
function () {
const authenticator = new PlainTextSaslAuthenticator('stephen',
'password');
-connection = helper.getDriverRemoteConnection(badServerAuthUrl, {
+connection = getDriverRemoteConnection(badServerAuthUrl, {
authenticator: authenticator,
- rejectUnauthorized: false
+ rejectUnauthorized: false,
});
-return connection.submit(new Bytecode().addStep('V',
[]).addStep('tail', []))
-.then(function() {
- assert.fail("server is running TLS and trying to connect with
ws:// so this should result in error thrown");
-})
-.catch(function (err) {
- if (err instanceof AssertionError) throw err;
- assert.ok(err);
- assert.ok(err.message === 'socket hang up');
-});
+return connection
+ .submit(new Bytecode().addStep('V', []).addStep('tail', []))
+ .then(function () {
+assert.fail('server is running TLS and trying to connect with
ws:// so this should result in error thrown');
+ })
+ .catch(function (err) {
+if (err instanceof AssertionError) throw err;
+assert.ok(err);
+assert.ok(err.message === 'socket hang up');
+ });
});
it('should return error when using ws:// for a TLS configured server',
function () {
const authenticator = new PlainTextSaslAuthenticator('stephen',
'password');
-connection = helper.getDriverRemoteConnection(badServerAuthUrl, {
+connection = getDriverRemoteConnection(badServerAuthUrl, {
authenticator: authenticator,
- rejectUnauthorized: false
+ rejectUnauthorized: false,
});
-const g = traversal().with_(connection);
-return g.V().toList().then(function() {
- assert.fail("server is running TLS and trying to connect with ws://
so this should result in error thrown");
-}).catch(function(err) {
-if (err instanceof AssertionError) throw err;
- assert.ok(err);
- assert.ok(err.message === 'socket hang up');
+const g = traversal().withRemote(connection);
Review Comment:
`withRemote` will be depricated in 4.0, `with_(` should be used.
the same thing a few more times
--
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-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
tien commented on PR #2515: URL: https://github.com/apache/tinkerpop/pull/2515#issuecomment-1992375619 > I think this is a good first step to implement the type system. Thank you for contribution @tien! > > Did I understand correctly that users will be able to use it without having to change the code? Is there any breaking changes? For most user, this will work just fine: ```ts ``` But for those that still use CommonJS instead of the standard ES Module. Then the import will be a bit weird: -- 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-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
vkagamlyk commented on PR #2515: URL: https://github.com/apache/tinkerpop/pull/2515#issuecomment-1992334497 I think this is a good first step to implement the type system. Thank you for contribution @tien! Did I understand correctly that users will be able to use it without having to change the code? Is there any breaking changes? -- 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-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
tien commented on PR #2515: URL: https://github.com/apache/tinkerpop/pull/2515#issuecomment-1989644763 > maybe just need to revive this one - https://issues.apache.org/jira/browse/TINKERPOP-2027 we didn't quite do what we thought we were going to do on that as i read back through that one. Yeah, I think that is relevant, have linked the mentioned ticket in the description, unless you guys think that I should create another one. -- 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-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
spmallette commented on PR #2515: URL: https://github.com/apache/tinkerpop/pull/2515#issuecomment-1989425590 maybe just need to revive this one - https://issues.apache.org/jira/browse/TINKERPOP-2027 we didn't quite do what we thought we were going to do on that as i read back through that one. -- 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-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
xiazcy commented on PR #2515: URL: https://github.com/apache/tinkerpop/pull/2515#issuecomment-1989420022 Thanks for the work @tien! I'm not familiar with JS or TS enough to provide insightful feedback, so I will leave that for folks who have the expertise. Just some housekeeping items, would you mind creating a ticket on [Jira](https://issues.apache.org/jira/projects/TINKERPOP/issues/) for this body of work, and link it into your PR description? It would help us track the discussions related to this for future. Thank you! -- 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-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat: TypeScript integration [tinkerpop]
codecov-commenter commented on PR #2515: URL: https://github.com/apache/tinkerpop/pull/2515#issuecomment-1985024642 ## [Codecov](https://app.codecov.io/gh/apache/tinkerpop/pull/2515?src=pr=h1_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) Report All modified and coverable lines are covered by tests :white_check_mark: > Project coverage is 76.51%. Comparing base [(`2d32517`)](https://app.codecov.io/gh/apache/tinkerpop/commit/2d32517b3bca1b00d716b3205c2abdbcd6ed3352?el=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) to head [(`e1aebb0`)](https://app.codecov.io/gh/apache/tinkerpop/pull/2515?src=pr=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache). > Report is 52 commits behind head on master. Additional details and impacted files ```diff @@ Coverage Diff @@ ## master#2515 +/- ## + Coverage 76.16% 76.51% +0.35% - Complexity1317013183 +13 Files 1085 1061 -24 Lines 6518961292-3897 Branches 7289 7298 +9 - Hits 4965146898-2753 + Misses1283011890 -940 + Partials 2708 2504 -204 ``` [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/tinkerpop/pull/2515?src=pr=continue_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache). :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral_source=github_content=comment_campaign=pr+comments_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-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] feat: TypeScript integration [tinkerpop]
tien opened a new pull request, #2515: URL: https://github.com/apache/tinkerpop/pull/2515 - Change public interfacing files to TypeScript - Keep internal serializers as JavaScript - Change CommonJS to Module -- 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-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
