Skip to content

Conversation

@jsjacob
Copy link
Contributor

@jsjacob jsjacob commented Jan 24, 2020

The current implementation of Record.getString() returns null for 0 and "" (empty string). I don't think this is the intended behavior.

This change makes the value test more explicit so 0 and "" are not returned as null.

@gkorland gkorland requested a review from DvirDukhan January 26, 2020 12:00
Copy link
Contributor

@DvirDukhan DvirDukhan left a comment

Choose a reason for hiding this comment

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

@jsjacob
Thanks for this PR
LGTM
can you please add a simple unit test for this case?

@jsjacob
Copy link
Contributor Author

jsjacob commented Feb 2, 2020

can you please add a simple unit test for this case?

I added several unit tests for Record.

@jsjacob jsjacob requested a review from DvirDukhan February 3, 2020 07:07
Copy link
Contributor

@DvirDukhan DvirDukhan left a comment

Choose a reason for hiding this comment

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

@jsjacob
Thanks!
I will merge it very soon

@DvirDukhan DvirDukhan merged commit 61ee61f into RedisGraph:master Feb 3, 2020
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.

2 participants