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

Remove support for manifest configuration #809

Merged
merged 2 commits into from
May 2, 2018
Merged

Remove support for manifest configuration #809

merged 2 commits into from
May 2, 2018

Conversation

Jawnnypoo
Copy link
Member

The ability to configure the server in the manifest is a sort of relic of when the SDK was being used with Parse.com. In order to move forward with allowing for customization, as well as adding new plugins such as FCM support, I think it is best that in the 1.17.0 release we ditch the Manifest configuration. This is the best time to do it, since 1.17.0 has API breaking changes which, if users are using GCM, will require them to update their Manifest and code anyhow.

We certainly would want to make sure it is clear in the release notes that this is occurring.

@codecov
Copy link

codecov bot commented May 1, 2018

Codecov Report

Merging #809 into master will decrease coverage by 1.11%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #809      +/-   ##
============================================
- Coverage     54.57%   53.46%   -1.12%     
+ Complexity     1716     1682      -34     
============================================
  Files           124      124              
  Lines          9874     9852      -22     
  Branches       1384     1380       -4     
============================================
- Hits           5389     5267     -122     
- Misses         4042     4156     +114     
+ Partials        443      429      -14
Impacted Files Coverage Δ Complexity Δ
Parse/src/main/java/com/parse/Parse.java 20.83% <100%> (-27.77%) 10 <0> (-12)
Parse/src/main/java/com/parse/ParsePlugins.java 25.27% <0%> (-16.49%) 7% <0%> (-10%)
...rse/src/main/java/com/parse/ParseCommandCache.java 0% <0%> (-7.19%) 0% <0%> (-4%)
...arse/src/main/java/com/parse/ParseCorePlugins.java 55.55% <0%> (-6.12%) 45% <0%> (-2%)
.../src/main/java/com/parse/ParseEventuallyQueue.java 0% <0%> (-5.48%) 0% <0%> (-3%)
Parse/src/main/java/com/parse/FileObjectStore.java 81.57% <0%> (-5.27%) 9% <0%> (ø)
...se/src/main/java/com/parse/ParseKeyValueCache.java 43% <0%> (-2%) 14% <0%> (-2%)
...in/java/com/parse/CachedCurrentUserController.java 90.51% <0%> (-0.87%) 14% <0%> (-1%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 329ef4a...d087211. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.2%) to 57.826% when pulling d087211 on Jawnnypoo:remove-manifest-config into 329ef4a on parse-community:master.

@Jawnnypoo Jawnnypoo requested a review from rogerhu May 1, 2018 04:19
.setTag(JOB_TAG_REGISTER) // uniquely identifies the job
.build();

Job job = ParseGCMJobService.createJob(dispatcher, gcmSenderFromManifest(context));
Copy link
Contributor

Choose a reason for hiding this comment

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

You're getting read of manifest but keeping it here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I tried to get rid of it for the GCM module, but it turns out we need the GCM sender ID at unpredictable times, such as when the token gets refreshed. So, we would either have to keep it in the manifest, or store it in SharedPreferences or something. I think it being in the manifest as it was before is the easiest configuration. Since most should be migrating away from the GCM module anyways, I think it is okay.

@rogerhu
Copy link
Contributor

rogerhu commented May 2, 2018

Ok we need to update https://docs.parseplatform.org/android/guide/ too...

@rogerhu rogerhu merged commit 2b84d8d into parse-community:master May 2, 2018
@Jawnnypoo Jawnnypoo deleted the remove-manifest-config branch May 2, 2018 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants