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

RNET-1133: Fixed equality comparison for collections in mixed #3573

Merged
merged 5 commits into from
Apr 16, 2024
Merged

Conversation

papafe
Copy link
Contributor

@papafe papafe commented Apr 16, 2024

Fixes #3572

@papafe papafe added the no-changelog Used to skip the changelog check label Apr 16, 2024
@papafe papafe self-assigned this Apr 16, 2024
@cla-bot cla-bot bot added the cla: yes label Apr 16, 2024
Copy link

coveralls-official bot commented Apr 16, 2024

Pull Request Test Coverage Report for Build 8706976682

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.02%) to 81.081%

Files with Coverage Reduction New Missed Lines %
Realm/Realm/DatabaseTypes/RealmValue.cs 1 92.25%
Totals Coverage Status
Change from base Build 8685639320: -0.02%
Covered Lines: 6794
Relevant Lines: 8235

💛 - Coveralls

Comment on lines +1536 to +1537
RealmValueType.List => AsList().Equals(other.AsList()),
RealmValueType.Dictionary => AsDictionary().Equals(other.AsDictionary()),
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Comment on lines 103 to 113
"string",
new byte[] { 0, 1, 2 },
new DateTimeOffset(1234, 5, 6, 7, 8, 9, TimeSpan.Zero),
1f,
2d,
3m,
new ObjectId("5f63e882536de46d71877979"),
Guid.Parse("3809d6d9-7618-4b3d-8044-2aa35fd02f31"),
new InternalObject { IntProperty = 10, StringProperty = "brown" },
innerList,
innerDict,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the loss of cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could have kept the full list, but in the end it shouldn't make a difference on the test and I thought it would make the test easier to read. I didn't really need to 😁

Copy link
Contributor

@nielsenko nielsenko Apr 16, 2024

Choose a reason for hiding this comment

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

Okay.. gut reaction was just that the coverage was diminished, and would those tests now have failed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's understandable, I'll put those back so it's easier to follow

Copy link
Contributor

@nielsenko nielsenko left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@rorbech rorbech left a comment

Choose a reason for hiding this comment

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

LGTM

@papafe papafe merged commit ea60689 into main Apr 16, 2024
66 of 67 checks passed
@papafe papafe deleted the fp/equality branch April 16, 2024 17:04
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes no-changelog Used to skip the changelog check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix equality comparison for collections in mixed
3 participants