-
-
Notifications
You must be signed in to change notification settings - Fork 145
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
Support for null values #160
Conversation
Codecov Report
@@ Coverage Diff @@
## main #160 +/- ##
==========================================
+ Coverage 57.48% 58.75% +1.26%
==========================================
Files 37 37
Lines 3262 3280 +18
==========================================
+ Hits 1875 1927 +52
+ Misses 1202 1166 -36
- Partials 185 187 +2
Continue to review full report at Codecov.
|
4ff5d6c
to
39ad65f
Compare
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.
I confirmed that it works with yorkie-js-sdk.
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.
LGTM! I left a comment for a minor thing
api/converter/converter_test.go
Outdated
@@ -67,6 +67,7 @@ func TestConverter(t *testing.T) { | |||
err := doc.Update(func(root *proxy.ObjectProxy) error { | |||
// an object and primitive types | |||
root.SetNewObject("k1"). | |||
SetNull("k.1.0"). |
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.
I'm curious about "k.1.0". 🤔 Is it different with "k1.0"?
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.
@mojosoeun No. It's just a key. I will change it to the same pattern as the other keys.
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.
LGTM 👍
Co-authored-by: Hyunwoo.Jo <[email protected]>
What this PR does / why we need it:
Support for null values.
Which issue(s) this PR fixes:
Fixes yorkie-team/yorkie-js-sdk#156
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation:
Checklist: