-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Make non-breaking changes to support arrays of primitives #5148
Conversation
b4fa6a9
to
da75922
Compare
Realm/RLMProperty.mm
Outdated
_type = RLMPropertyTypeArray; | ||
_objectClassName = value.objectClassName; | ||
if (_objectClassName) { | ||
// FIXME: validate |
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.
Is this an existing FIXME that was moved around, or something new that should be addressed?
@@ -161,7 +161,7 @@ - (void)testDynamicTypes { | |||
|
|||
// verify properties | |||
RLMRealm *dyrealm = [self realmWithTestPathAndSchema:nil]; | |||
RLMResults *results = [dyrealm allObjects:AllTypesObject.className]; | |||
RLMResults<RLMObject *> *results = [dyrealm allObjects:AllTypesObject.className]; |
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.
Are these changes required by this PR, or are they being made preemptively in advance of #5149?
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.
All of the tests build and pass with no changes to the tests other than updating what error message a few of the tests are looking for.
@@ -82,13 +82,13 @@ class SwiftRecursingSchemaTestObject : RLMObject { | |||
} | |||
|
|||
class InitAppendsToArrayProperty : RLMObject { | |||
@objc dynamic var propertyWithIllegalDefaultValue: RLMArray = { | |||
@objc dynamic var propertyWithIllegalDefaultValue: RLMArray<SwiftIntObject> = { |
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.
Are these changes required by this PR, or are they being made preemptively in advance of #5149?
@@ -80,7 +80,7 @@ extension Realm { | |||
instance provides access to the old and new database schemas, the objects in the Realm, and provides functionality for | |||
modifying the Realm during the migration. | |||
*/ | |||
public final class Migration { | |||
public struct Migration { |
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 assume that changing from class to struct doesn't in any way change how users interact with these types (i.e., it's not a breaking change)?
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 don't see any way for it to break anything, since they're immutable types which the user can't construct directly. Realm
should maybe also be a struct but that would actually be a breaking change.
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.
Users can theoretically do stupid things like retroactively conforming Migration
to a class-only protocol, but I don't think that's something we should worry about.
da75922
to
c902e67
Compare
The custom failure messages actually made it harder to figure out what had gone wrong than the default message.
…between NSNumber and RLMArray
These are all immutable and cheap to copy, so there's no reason for them to be reference types.
d95d98e
to
25c62de
Compare
@@ -114,7 +114,8 @@ + (instancetype)schemaForObjectClass:(Class)objectClass { | |||
Class superClass = class_getSuperclass(cls); | |||
NSArray *allProperties = @[]; | |||
while (superClass && superClass != RLMObjectBase.class) { | |||
allProperties = [[RLMObjectSchema propertiesForClass:cls isSwift:isSwift] arrayByAddingObjectsFromArray:allProperties]; | |||
allProperties = [[RLMObjectSchema propertiesForClass:cls isSwift:isSwift] | |||
arrayByAddingObjectsFromArray:allProperties]; |
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.
indentation
Realm/RLMObjectSchema.mm
Outdated
} | ||
|
||
// Check to see if this optional property is an underlying storage property for a Swift lazy var. | ||
// Managed lazy vars are't allowed. |
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.
"aren't"
A bunch of individually not very interesting changes made to support arrays of primitives. Should have no functional changes other than some error messages being a bit different.
Pulls in realm/realm-object-store#464 (and so depends on that getting merged).