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

kn/asymmetric sync #1400

Merged
merged 10 commits into from
Sep 18, 2023
Merged

kn/asymmetric sync #1400

merged 10 commits into from
Sep 18, 2023

Conversation

nielsenko
Copy link
Contributor

@nielsenko nielsenko commented Sep 14, 2023

Support asymmetric objects

@RealmModel(ObjectType.asymmetricObject)
class _Asymmetric {
  @PrimaryKey()
  @MapTo('_id')
  late ObjectId id;

  late List<_Embedded> embeddedObjects;
}
// ...    
realm.write(() {
  realm.ingest(Asymmetric(oid));
});

Fixes: #917

@cla-bot cla-bot bot added the cla: yes label Sep 14, 2023
@nielsenko nielsenko changed the base branch from main to kn/update-analyzer September 14, 2023 16:47
@nielsenko nielsenko force-pushed the kn/asymmetric-sync branch 2 times, most recently from 1d36c01 to abaa322 Compare September 15, 2023 10:27
@nielsenko
Copy link
Contributor Author

nielsenko commented Sep 15, 2023

Note that this PR is based #1365 which is again dependent on dart 3.1.0 which currently breaks linux due to: dart-lang/sdk#53267. Fix is planned to be cherry-picked: dart-lang/sdk#53503.

@nielsenko nielsenko marked this pull request as ready for review September 15, 2023 10:53
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.

left some small comments. I think its good. Should we have a final discussion on realm.ingest vs realm.add before we merge?

lib/src/realm_class.dart Outdated Show resolved Hide resolved
test/asymmetric_test.dart Outdated Show resolved Hide resolved
@nielsenko nielsenko changed the base branch from kn/update-analyzer to kn/use-analyzer-5.13 September 15, 2023 13:54
@nielsenko
Copy link
Contributor Author

left some small comments. I think its good. Should we have a final discussion on realm.ingest vs realm.add before we merge?

Yes, I cannot make up my mind myself. I'm okay with both, but it cannot be as elegant as in realm-dotnet since we cannot overload add. Let us discuss Monday.

Base automatically changed from kn/use-analyzer-5.13 to main September 15, 2023 15:40
@coveralls-official
Copy link

coveralls-official bot commented Sep 15, 2023

Pull Request Test Coverage Report for Build 6220147351

  • 30 of 34 (88.24%) changed or added relevant lines in 6 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.01%) to 89.045%

Changes Missing Coverage Covered Lines Changed/Added Lines %
generator/lib/src/field_element_ex.dart 0 1 0.0%
lib/src/native/realm_core.dart 0 1 0.0%
lib/src/realm_object.dart 1 3 33.33%
Files with Coverage Reduction New Missed Lines %
generator/lib/src/pseudo_type.dart 1 44.44%
Totals Coverage Status
Change from base Build 6200070966: 0.01%
Covered Lines: 3357
Relevant Lines: 3770

💛 - Coveralls

@nielsenko
Copy link
Contributor Author

nielsenko commented Sep 16, 2023

left some small comments. I think its good. Should we have a final discussion on realm.ingest vs realm.add before we merge?

Yes, I cannot make up my mind myself. I'm okay with both, but it cannot be as elegant as in realm-dotnet since we cannot overload add. Let us discuss Monday.

I have created a commit that illustrates how it could be implemented, if we go with add

@nielsenko nielsenko force-pushed the kn/asymmetric-sync branch 3 times, most recently from cf7e1af to ee32ce2 Compare September 16, 2023 14:05
@nielsenko
Copy link
Contributor Author

left some small comments. I think its good. Should we have a final discussion on realm.ingest vs realm.add before we merge?

Yes, I cannot make up my mind myself. I'm okay with both, but it cannot be as elegant as in realm-dotnet since we cannot overload add. Let us discuss Monday.

For now we will go with ingest. I have parked the refactoring commit that overload add on the kn/asymmetric-add-instead-of-ingest branch

@nielsenko nielsenko merged commit d11aefa into main Sep 18, 2023
33 checks passed
@nielsenko nielsenko deleted the kn/asymmetric-sync branch September 18, 2023 08:49
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Asymmetric Sync
3 participants