-
Notifications
You must be signed in to change notification settings - Fork 89
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
Implement indexOf using realm_list_find #911
Conversation
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.
Looks good, two minor things from me.
d6d4a9e
to
73e76b9
Compare
Pull Request Test Coverage Report for Build 3097300398
💛 - Coveralls |
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 think we need to move the throwing of the Error in a more obvious place. Now it's really indirect which makes the validation logic hard to follow
79a8161
to
f5794dc
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.
Lets handle the comments about the place where we throw the RealmStateError
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.
Be sure to be merged with core [v12.7.0] coming with PR Fix list query bug before to push into master.
Includes: 4d21f9a87 Expose `list_find` in the c api (#5848)
This both simplifies and optimize the implementation of UnmanagedRealmList, as it delegates all operation to the backing list.
f5794dc
to
e474dae
Compare
Sure |
Done |
d6d7a81
to
1adcb09
Compare
test/list_test.dart
Outdated
realm.write(() => realm.add(team)); | ||
final players = team.players; | ||
|
||
// expect(players, isA<ManagedRealmList<Person>>()); |
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.
Delete it if not needed.
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.
We should let the null error bubble up and crash the app every time.
Fixes #910