-
Notifications
You must be signed in to change notification settings - Fork 88
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
DOCSP-35364: Flutter SDK - Nested Collections of Mixed Data #3201
DOCSP-35364: Flutter SDK - Nested Collections of Mixed Data #3201
Conversation
Readability for Commit Hash: a97a303 You can see any previous Readability scores (if they exist) by looking Readability scores for changed documents:
For Grade Level, aim for 8 or below. For Reading Ease scores, aim for 60 or above:
For help improving readability, try Hemingway App. |
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 haven't gone through the whole thing yet since it's quite late, but I'll get to it on Monday! I think the structure is sound, but we could come up with better examples. There are a few inaccuracies as well that I've highlighted.
source/sdk/flutter/realm-database/model-data/define-realm-object-schema.txt
Outdated
Show resolved
Hide resolved
source/sdk/flutter/realm-database/model-data/define-realm-object-schema.txt
Outdated
Show resolved
Hide resolved
...ples/generated/flutter/define_realm_model_test.snippet.create-unstructured-data-example.dart
Outdated
Show resolved
Hide resolved
source/examples/generated/flutter/data_types_test.snippet.realm-value-nested-collections.dart
Outdated
Show resolved
Hide resolved
source/examples/generated/flutter/data_types_test.snippet.realm-value-nested-collections.dart
Outdated
Show resolved
Hide resolved
source/examples/generated/flutter/data_types_test.snippet.realm-value-nested-collections.dart
Outdated
Show resolved
Hide resolved
source/examples/generated/flutter/data_types_test.snippet.realm-value-nested-collections.dart
Outdated
Show resolved
Hide resolved
3351c2a
to
bc24393
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.
Overall, content looks good! A few smaller suggestions and fixes.
For code snippets, I would like to see introduction comments to explain what the code is doing. Sometimes you explain what the code is doing in the text, sometimes not. I think an introduction comment is helpful whether the code is explained in text or not, given the code is copyable.
in the nested data and to update specific elements, but it is less | ||
performant than using a structured schema or serializing JSON blobs |
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.
in the nested data and to update specific elements, but it is less | |
performant than using a structured schema or serializing JSON blobs | |
in the nested data and to update specific elements. However in some cases, it is less | |
performant than using a structured schema or serializing JSON blobs. |
Why it is less performant? Maybe give some reasons or cases where it would be less performant.
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've noted this for a fast follow addition. I'll need to get more info from engineering for the additional details
source/sdk/flutter/realm-database/model-data/define-realm-object-schema.txt
Show resolved
Hide resolved
5e07cea
to
5541428
Compare
5541428
to
7cf1966
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.
Some typos and more stuff about comments, add at your discretion. Otherwise, LGTM!
source/sdk/flutter/realm-database/model-data/define-realm-object-schema.txt
Show resolved
Hide resolved
ce22f63
to
50f7a4b
Compare
Co-authored-by: Nikola Irinchev <irinchev@me.com>
50f7a4b
to
25bafda
Compare
✨ Staging URL: https://preview-mongodbmongodb.gatsbyjs.io/realm/master/ 🪵 Logs |
Pull Request Info
Jira ticket: https://jira.mongodb.org/browse/DOCSP-35364
RealmValue
mixed data type section with new info and examples for collections in mixed; added note re. breaking change (links to migration page from a separate PR)Reminder Checklist
Before merging your PR, make sure to check a few things.
Release Notes
RealmValue
section updated to document the new support for nested collections of mixed data and additional changes to the data type. Tested, Bluehawked code examples were updated or added.Review Guidelines
REVIEWING.md