-
-
Notifications
You must be signed in to change notification settings - Fork 735
Decouple object subclassing. #245
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
Conversation
By analyzing the blame information on this pull request, we identified @grantland to be a potential reviewer. |
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.
Similar to the state we have on .NET, we lose this logic during this decouple. Thoughts on a good cross-platform and extensible way to have a hook when a certain class is unregistered?
Also: This becomes irrelevant with automatic subclass registration, which is coming soon™
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.
Could we keep this for now with a TODO to move to a hook?
The only clean way (in API terms) would be to add listeners/callbacks to ParseObjectSubclassingController
for unregister that ParseUser
can subscribe to, but that involves a bunch of class allocations...
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.
ping
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.
Yep, already implemented in fact: https://github.com/ParsePlatform/Parse-SDK-Android/pull/245/files#diff-d1fc53b1cc8d9fefba8e9a00706881e1R104
057e92f
to
90ea274
Compare
@richardjrossiii updated the pull request. |
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.
did we lose this case?
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.
That specific message? Yes. That error case is still handled by isTypeValid()
, however. Without leaking abstraction I'm not sure how we can keep both error messages.
Tests should pass again. |
@richardjrossiii updated the pull request. |
Ignore facebook bot. it's drunk. |
This fully decouples object subclassing from the ParseObject class, bringing it up to parity with our other native platforms.
dfc1ab1
to
d4d9305
Compare
@richardjrossiii updated the pull request. |
LGTM! |
…couple Decouple object subclassing.
@richardjrossiii updated the pull request. |
This fully decouples object subclassing from the ParseObject class, bringing it up to parity with our other native platforms.