Skip to content

Livequery - #306

Merged
phpnode merged 4 commits into
codemix:masterfrom
luigidellaquila:livequery
May 19, 2015
Merged

Livequery#306
phpnode merged 4 commits into
codemix:masterfrom
luigidellaquila:livequery

Conversation

@luigidellaquila

Copy link
Copy Markdown
Contributor

Hi @phpnode

this is a pull request for live query support.
Please review this (especially connection.js line 360...) and if you need any info just let me know

Thanks a lot

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

obj is defined twice in this function so the tests are failing, I would remove the obj declaration and replace this with:

return [
  Operation.LIVE_RESULT,
  token,
  operation,
  {
    content: content,
    type: 'd',
    cluster: clusterId,
    position: clusterPosition,
    version: version
  },
  offset
];

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Charles,

thank you very much, I'll fix it and will send another PR soon

Thanks

Luigi

2015-04-27 20:42 GMT+02:00 Charles Pick notifications@github.com:

In lib/transport/binary/protocol28/operation.js
#306 (comment):

  •    offset+=4;
    
  •    var clusterId = buffer.readInt16BE(offset);
    
  •    offset+=2;
    
  •    var clusterPosition = Long.fromBits(
    
  •        buffer.readUInt32BE(offset + 4),
    
  •        buffer.readInt32BE(offset)
    
  •    ).toNumber();
    
  •    offset+=8;
    
  •    var contentLenght = buffer.readInt32BE(offset);
    
  •    offset+=4;
    
  •    var asString = buffer.toString('utf8', offset, offset + contentLenght);
    
  •    offset += contentLenght;
    
  •    var content = deserializer.deserialize(asString, this.data.transformerFunctions);
    
  •    var obj = {};
    

obj is defined twice in this function so the tests are failing, I would
remove the obj declaration and replace this with:

return [
Operation.LIVE_RESULT,
token,
operation,
{
content: content,
type: 'd',
cluster: clusterId,
position: clusterPosition,
version: version
},
offset
];


Reply to this email directly or view it on GitHub
https://github.com/codemix/oriento/pull/306/files#r29177038.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Charles,

Fix applied, let me know if the rest is ok

Thank you very much!

Luigi

2015-04-28 11:29 GMT+02:00 Luigi Dell'Aquila luigi.dellaquila@gmail.com:

Hi Charles,

thank you very much, I'll fix it and will send another PR soon

Thanks

Luigi

2015-04-27 20:42 GMT+02:00 Charles Pick notifications@github.com:

In lib/transport/binary/protocol28/operation.js
#306 (comment):

  •    offset+=4;
    
  •    var clusterId = buffer.readInt16BE(offset);
    
  •    offset+=2;
    
  •    var clusterPosition = Long.fromBits(
    
  •        buffer.readUInt32BE(offset + 4),
    
  •        buffer.readInt32BE(offset)
    
  •    ).toNumber();
    
  •    offset+=8;
    
  •    var contentLenght = buffer.readInt32BE(offset);
    
  •    offset+=4;
    
  •    var asString = buffer.toString('utf8', offset, offset + contentLenght);
    
  •    offset += contentLenght;
    
  •    var content = deserializer.deserialize(asString, this.data.transformerFunctions);
    
  •    var obj = {};
    

obj is defined twice in this function so the tests are failing, I would
remove the obj declaration and replace this with:

return [
Operation.LIVE_RESULT,
token,
operation,
{
content: content,
type: 'd',
cluster: clusterId,
position: clusterPosition,
version: version
},
offset
];


Reply to this email directly or view it on GitHub
https://github.com/codemix/oriento/pull/306/files#r29177038.

@luigidellaquila

Copy link
Copy Markdown
Contributor Author

Hi @phpnode

I just applied the suggested fix. Let me know if the rest is ok

Thanks

Luigi

phpnode added a commit that referenced this pull request May 19, 2015
@phpnode
phpnode merged commit e919717 into codemix:master May 19, 2015
@phpnode

phpnode commented May 19, 2015

Copy link
Copy Markdown
Member

@luigidellaquila thanks!

@gauravdhiman

Copy link
Copy Markdown

Can you kindly update the Readme with an example to use merged feature of LiveQuery.
I would like to use the LiveQuery feature for building Meteor package to make Meteor and OrientDB work together.

@phpnode

phpnode commented May 20, 2015

Copy link
Copy Markdown
Member

@luigidellaquila please could you open another PR with that? :)

@luigidellaquila

Copy link
Copy Markdown
Contributor Author

Of course I will ;-)

Thanks

Luigi

2015-05-20 15:57 GMT+02:00 Charles Pick notifications@github.com:

@luigidellaquila https://github.com/luigidellaquila please could you
open another PR with that? :)


Reply to this email directly or view it on GitHub
#306 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants