-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
allow "nothing" returns for evalscript #110
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #110 +/- ##
==========================================
- Coverage 82.76% 82.60% -0.16%
==========================================
Files 9 9
Lines 412 414 +2
==========================================
+ Hits 341 342 +1
- Misses 71 72 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like evalscript(conn, "return {10,20}", 0, [1,2])
still fails. Not sure whether we need to convert the response to string at all.
Was discussing this with @aviks. I will add a commit here, removing the conversion step. Will also add some tests. |
Similar to the other pending change, this would be a breaking change that requires a major version bump. That isn't necessarily a problem, but if we're going to be making breaking changes, we should bundle them up together and make them all at once. |
Yes, I think we can plan a release only after we get both those changes in. |
I guess we can merge this now? Will do in a bit. |
Incrementing the major version, preparing for release. Ref: #110 (comment)
Without this, a method error is thrown when evalscript has an empty return