Skip to content

Add better, more flexible APIs for SDK initialization. #284

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 12, 2015
Merged

Add better, more flexible APIs for SDK initialization. #284

merged 1 commit into from
Dec 12, 2015

Conversation

richardjrossiii
Copy link
Contributor

This PR includes public API changes

Proposing a new initialization API, to allow for more flexibility and clarity in the future:

Parse.Initialize(new Parse.Configuration.Builder(context)
    .applicationId(...)
    .clientKey(...)
    .build()
);

No APIs are being directly deprecated at this time, only additions for the time being.

Equivalent PRs for .NET: parse-community/Parse-SDK-dotNET#79, and iOS: parse-community/Parse-SDK-iOS-OSX#570.

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.

@facebook-github-bot
Copy link

By analyzing the blame information on this pull request, we identified @grantland and @wangmengyan95 to be potential reviewers.

grantland
grantland reviewed Dec 8, 2015
View reviewed changes
Parse/src/main/java/com/parse/Parse.java
this.interceptors = builder.interceptors;
}

public static final class Builder {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: class definitions above instance

@grantland
Copy link
Contributor

documentation? tests?

@richardjrossiii
Copy link
Contributor Author

Similar to other platforms, documentation forthcoming after we finalize APIs across platforms. I'll add a test or two though.

@richardjrossiii richardjrossiii force-pushed the richardross.initialization.update branch 2 times, most recently from b640d90 to f3e78cc Compare December 10, 2015 19:03
@facebook-github-bot
Copy link

@richardjrossiii updated the pull request.

@richardjrossiii
Copy link
Contributor Author

Tests still forthcoming, fixed nits.

grantland
grantland reviewed Dec 11, 2015
View reviewed changes
Parse/src/main/java/com/parse/Parse.java
@@ -46,6 +110,8 @@
private static boolean isLocalDatastoreEnabled;
private static OfflineStore offlineStore;

private static Configuration configuration;
Copy link
Contributor

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 to hold this, we can just pass in configuration.interceptors through to initializeParseHttpClientsWithParseNetworkInterceptors()

@facebook-github-bot
Copy link

@richardjrossiii updated the pull request.

grantland
grantland reviewed Dec 11, 2015
View reviewed changes
Parse/src/test/java/com/parse/ParseClientConfigurationTest.java

try {
configurationA.interceptors.add(interceptorB);
fail();
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a message here "interceptors should not be mutable"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@grantland
Copy link
Contributor

LGTM, one nit

@grantland grantland removed their assignment Dec 11, 2015
@richardjrossiii
Add better, more flexible APIs for SDK initialization.
7cec3b4 
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.
@richardjrossiii richardjrossiii force-pushed the richardross.initialization.update branch from 8cea8da to 7cec3b4 Compare December 11, 2015 22:44
@richardjrossiii
Copy link
Contributor Author

Added message, squashed, and good to go! Will merge once travis passes.

@facebook-github-bot
Copy link

@richardjrossiii updated the pull request.

richardjrossiii added a commit that referenced this pull request Dec 12, 2015
@richardjrossiii
Merge pull request #284 from ParsePlatform/richardross.initialization…
8542d98 
….update

Add better, more flexible APIs for SDK initialization.
@richardjrossiii richardjrossiii merged commit 8542d98 into master Dec 12, 2015
@richardjrossiii richardjrossiii deleted the richardross.initialization.update branch December 12, 2015 00:55
@grantland grantland modified the milestone: 1.11.1 Dec 17, 2015
@facebook-github-bot
Copy link

@richardjrossiii updated the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@richardjrossiii richardjrossiii

Labels
None yet
Projects
None yet
Milestone
1.12.0
Development

Successfully merging this pull request may close these issues.
4 participants
@richardjrossiii @facebook-github-bot @grantland @rogerhu