Skip to content

Conversation

@DvirDukhan
Copy link
Contributor

No description provided.

@DvirDukhan DvirDukhan requested a review from gkorland December 16, 2019 16:11
@codecov
Copy link

codecov bot commented Dec 16, 2019

Codecov Report

Merging #31 into master will increase coverage by 0.37%.
The diff coverage is 76.74%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #31      +/-   ##
==========================================
+ Coverage   90.09%   90.47%   +0.37%     
==========================================
  Files           6        6              
  Lines         101      105       +4     
  Branches        6        6              
==========================================
+ Hits           91       95       +4     
  Misses         10       10
Impacted Files Coverage Δ
src/label.js 100% <ø> (ø) ⬆️
src/record.js 100% <ø> (ø) ⬆️
src/statistics.js 100% <100%> (ø) ⬆️
src/path.js 72.72% <71.42%> (ø) ⬆️
src/node.js 77.77% <75%> (ø) ⬆️
src/edge.js 81.81% <80%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 623b5de...d321ab6. Read the comment docs.

@DvirDukhan DvirDukhan force-pushed the comply_to_1.99.7_index_response branch 4 times, most recently from 53c277b to ccbebf8 Compare December 17, 2019 18:50
},
"dependencies": {
"redisgraph.js": "^1.2.0"
"redisgraph.js": "github:RedisGraph/redisgraph.js"
Copy link
Contributor

Choose a reason for hiding this comment

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

it might be an issue if someone will copy it, you might want to set a relative path

src/graph.js Outdated
* @param port Redis port
* @param options node_redis options
* @param {string} graphId the graph id
* @param {*} host Redis host or node_redis client
Copy link
Contributor

Choose a reason for hiding this comment

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

{(string|redis}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

src/graph.js Outdated
* @param options node_redis options
* @param {string} graphId the graph id
* @param {*} host Redis host or node_redis client
* @param {*} port Redis port
Copy link
Contributor

Choose a reason for hiding this comment

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

why {*} I guess you would like to mark as optional

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

src/record.js Outdated
/**
* Builds a Record object
* @constructor
* @param {*} header
Copy link
Contributor

Choose a reason for hiding this comment

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

why header not typed ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

src/resultSet.js Outdated
* A raw representation of a header (query response schema) is a list.
* Each entry in the list is a tuple (list of size 2).
* tuple[0] represents the type of the column, and tuple[1] represents the name of the column.
* @param rawHeader
Copy link
Contributor

Choose a reason for hiding this comment

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

type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

src/resultSet.js Outdated
// [[name, value, value type] X N]
/**
* Parse raw entity properties representation into a Map
* @param {*} props
Copy link
Contributor

Choose a reason for hiding this comment

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

why {*} ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@DvirDukhan DvirDukhan force-pushed the comply_to_1.99.7_index_response branch 2 times, most recently from 6e5d2d2 to 4ebd238 Compare December 26, 2019 12:47
@DvirDukhan DvirDukhan force-pushed the comply_to_1.99.7_index_response branch from 4ebd238 to d321ab6 Compare December 26, 2019 12:48
@gkorland gkorland merged commit 93096f4 into master Dec 26, 2019
@gkorland gkorland deleted the comply_to_1.99.7_index_response branch December 26, 2019 16:10
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