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.

Sign up for GitHub

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

Jump to bottom

Expose embeddedObject.getParent #979

Merged
merged 5 commits into from
Nov 3, 2022
Merged

Expose embeddedObject.getParent #979

merged 5 commits into from
Nov 3, 2022

Conversation

nirinchev
Copy link
Member

This is a follow up on the embedded objects PR that I pulled to a separate branch because it depends on realm/realm-core#5960.

@cla-bot cla-bot bot added the cla: yes label Oct 20, 2022
desistefanova
desistefanova reviewed Oct 21, 2022
View reviewed changes
Copy link
Contributor

@desistefanova desistefanova left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to update the core version in dart Changelog.

desistefanova
desistefanova reviewed Oct 21, 2022
View reviewed changes
Copy link
Contributor

@desistefanova desistefanova left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think having these new functions can help us to implement DynamicRealm find and all to support embedded objects?
Or it must be similar to Realm class and to support only RealmObjects?

@nirinchev
Copy link
Member Author

They should be equivalent - i.e. we should not allow users to lookup embedded objects directly.

Base automatically changed from ni/embedded-objects-2 to master October 21, 2022 14:03
nielsenko
nielsenko reviewed Nov 3, 2022
View reviewed changes
lib/src/realm_object.dart Outdated Show resolved Hide resolved
@nielsenko nielsenko force-pushed the ni/embedded-parent branch 2 times, most recently from 62d5da6 to 3f6289e Compare November 3, 2022 12:06
@coveralls
Copy link

coveralls commented Nov 3, 2022 •
edited
Loading

Pull Request Test Coverage Report for Build 3387706820

  • 16 of 19 (84.21%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.08%) to 89.594%
Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/src/realm_class.dart 3 6 50.0%
Files with Coverage Reduction New Missed Lines %
lib/src/subscription.dart 1 90.48%
Totals Coverage Status
Change from base Build 3385625020: -0.08%
Covered Lines: 2669
Relevant Lines: 2979
💛 - Coveralls

blagoev
blagoev reviewed Nov 3, 2022
View reviewed changes
Copy link
Contributor

@blagoev blagoev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's ok but I have the feeling we missed something while rebasing. Since this getObjectKey does not seem to be used anywhere

lib/src/realm_object.dart Outdated Show resolved Hide resolved
lib/src/realm_object.dart Outdated Show resolved Hide resolved
lib/src/util.dart Outdated Show resolved Hide resolved
lib/src/native/realm_core.dart Outdated Show resolved Hide resolved
@blagoev
Copy link
Contributor

blagoev commented Nov 3, 2022

should have a changelog entry

blagoev
blagoev suggested changes Nov 3, 2022
View reviewed changes
Copy link
Contributor

@blagoev blagoev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we check the rebase and also add a changelog entry. Just making it more visible here

@blagoev blagoev merged commit ddb98cf into master Nov 3, 2022
@blagoev blagoev deleted the ni/embedded-parent branch November 3, 2022 17:43
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@blagoev blagoev blagoev approved these changes

@nielsenko nielsenko nielsenko approved these changes

@desistefanova desistefanova desistefanova approved these changes

Assignees

@nirinchev nirinchev

Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@nirinchev @coveralls @blagoev @nielsenko @desistefanova