Skip to content
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

feat: added RawValue interface #127

Merged
merged 3 commits into from
Mar 19, 2021
Merged

feat: added RawValue interface #127

merged 3 commits into from
Mar 19, 2021

Conversation

nrwiersma
Copy link
Contributor

To allow for the detection of a nil return, a new interface is added (to maintain backwards compatibility) and implemented by value. This interface, RawValue, allows the retrieval of the raw bytes but requires a type assertion.

Fixes #126

To allow for the detection of a `nil` return, a new interface is added (to maintain backwards compatibility) and implemented by `value`. This interface, `RawValue`, allows the retrieval of the raw bytes but requires a type assertion.
@timcolson
Copy link

I appreciate the PR, @nrwiersma. I assume it's an improvement, but as a NewB to Lorca, I'm curious about the reason for it. What happens currently when a nil is returned? (from JS, yes?) Thx!

@nrwiersma
Copy link
Contributor Author

@timcolson If you call To(interface{}) when it is nil you get some fairly nasty errors.

@timcolson
Copy link

Thx @nrwiersma, I appreciate the context. Surface of change seems minimal, so curious to see if @zserge will merge it in. 👍

@zserge
Copy link
Owner

zserge commented Mar 19, 2021

I have no objections against the Bytes() method. But I think that adding it to the existing Value interface would still be backwards-compatible (adding methods should be fine, unlike removing/renaming them). I double anyone implements this interface outside of Lorca. And it should simplify the calling code.

@zserge zserge merged commit 4d6397d into zserge:master Mar 19, 2021
@nrwiersma nrwiersma deleted the raw-value branch March 19, 2021 12:03
@timcolson
Copy link

Yay! Another merge. Woot! 👍

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.

No way of knowing when Value is nil
3 participants