-
-
Notifications
You must be signed in to change notification settings - Fork 873
Add better, more flexible APIs for SDK initialization. #570
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
Parse/ParseClientConfiguration.h
Outdated
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.
Nullability annotate, pretty please. 😁
Parse/ParseClientConfiguration.h
Outdated
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.
Nit: Space before BOOL
1b588bf to
2497c8b
Compare
Parse/ParseClientConfiguration.h
Outdated
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 think we need these, simply due to the fact that you can't init it anyway after allocating... 2 less methods polluting the name space?
2497c8b to
2951556
Compare
2951556 to
94492ca
Compare
|
Hey all, just added some preliminary APIs for configuring URL sessions & retry attempts that we use in the SDK. Any feedback would be great! |
|
@richardjrossiii updated the pull request. |
|
Hi @richardjrossiii When do you think this PR could be merged into master? |
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 never realized that iOS uses set instead of initialize. I do like moving towards initialize since it's not exactly a setter, but is it weird to have it be named differently?
Also, if we keep initialize, we should probably update the abstract of it from Sets the configuration to be used for parse to Initialize Parse with a configuration or something
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.
My best guess for the reason that we use set instead of initialize on iOS - +initialize is a method declared by NSObject, equivalent to a static contractor in java-land.
It would have caused some ambiguity for developers if we had a method call like [Parse initialize:@"appKey" clientKey:@"ck"], but we can get around that now by marking +initialize as unavailable for use (which wasn't possible when Parse was first created).
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.
Ah that makes sense, SGTM!
|
Somewhat nitpicky, but |
Yup, it's definitely confusing. It's ObjC anyway, so long names are expected... Another potential name that could work is |
Parse/ParseClientConfiguration.m
Outdated
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.
Rename to initWithBlock:, as you don't have it in the static constructor.
|
@richardjrossiii updated the pull request. |
Parse/Internal/ParseManager.m
Outdated
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.
Not sure if this is the appropriate place for it, as startManaging doesn't quite relate to Offline Store only.
|
@richardjrossiii updated the pull request. |
|
@richardjrossiii updated the pull request. |
Parse/ParseClientConfiguration.m
Outdated
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.
Maybe invoke -initEmpty here and declare that as internal designated initializer (class extension in the .m file). Don't feel strongly about it, tho.
|
One small nit, otherwise - it's perfect! |
93bf830 to
61c2f8d
Compare
|
@richardjrossiii updated the pull request. |
Our current SDK initialization is lackluster in that it's split across multiple classes, files, and method calls. This PR hopes to unify some of these initialization settings int o a single 'configuration' class that can be easily used to intialize parse in a much cleaner way, especially as we add even more initialization options.
61c2f8d to
f43a96e
Compare
….update Add better, more flexible APIs for SDK initialization.
|
Hi @richardjrossiii |
|
@oferRounds No, not yet, this is going to be included in |
|
@richardjrossiii - cool, thanks! |
|
@richardjrossiii updated the pull request. |
|
The label |
This PR includes public API Changes
Old initialization looked somewhat outdated, our new APIs allow a usage similar to this:
ObjC
Swift
We believe that this provides a much cleaner interface for initialization. Please feel free to comment below on any feedback that you may have on this
Our current SDK initialization is lackluster in that it's split across multiple classes, files, and method calls.
This PR hopes to unify some of these initialization settings int o a single 'configuration' class that can be easily used to intialize parse in a much cleaner way, especially as we add even more initialization options.