Update everything, remove deprecated stuff from updates, make Travis builds less flaky, and update storage sample #559
Conversation
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
…on unification Signed-off-by: Alex Saveau <[email protected]>
|
CLAs look good, thanks! |
| @@ -347,9 +346,7 @@ public Void then(@NonNull Task<GoogleApiClient> task) throws Exception { | |||
| }); | |||
|
|
|||
| // Facebook sign out | |||
| if (FacebookSdk.isInitialized()) { | |||
There was a problem hiding this comment.
Did something change the Facebook SDK that we no longer need to deal with initialization?
There was a problem hiding this comment.
@samtstern Since you noticed, I might as well explain it now instead of in my review. The easiest would be to read Facebook's release notes: https://developers.facebook.com/docs/android/change-log-4x#4_19_0 and the upgrade guide https://developers.facebook.com/docs/android/upgrading-4x#4180to4190.
Signed-off-by: Alex Saveau <[email protected]>
SUPERCILEX
changed the title
|
@samtstern If everything looks good to you, both Facebook login and the storage permission stuff work great so this should be good to merge once the Travis build passes. |
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
|
@samtstern Urgh. It should be fixed in f501d20 though! |
| @@ -19,6 +19,10 @@ | |||
| android:name="com.facebook.sdk.ApplicationId" | |||
| android:value="@string/facebook_application_id"/> | |||
|
|
|||
| <meta-data | |||
There was a problem hiding this comment.
@SUPERCILEX Could we be inadvertently disabling something in developer's apps? What if they have a conflicting value in their AndroidManifest?
There was a problem hiding this comment.
@samtstern cc4b64b is the best solution I can come up with. Since we are upgrading the Facebook sdk for people, they will expect the Facebook upgrade guide (remove the initializeApp) to work. With this latest commit, that will still work, but people will have to override our manifest if they want to disable it:
<meta-data
android:name="com.facebook.sdk.AutoLogAppEventsEnabled"
android:value="false"
tools:replace="android:value"/>Signed-off-by: Alex Saveau <[email protected]>
auth/build.gradle
Outdated
| minifyEnabled false | ||
| proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.pro' | ||
| } | ||
|
|
||
| debug { | ||
| manifestPlaceholders = [enableFbLogging: false] |
There was a problem hiding this comment.
This is fine, just add a comment here saying this is only for testing. Since in the real world library modules are always compiled in their 'release' variant. So our end users will see enableFbLogging=true always
|
@SUPERCILEX the manifest placeholders may not be working in the Robolectric tests, the tests are unhappy again. |
|
@samtstern Yeah, I noticed. I thought tests ran in debug mode, but I guess not. I think I'll just have to copy the manifest for the tests. |
|
@samtstern I figured it out: |
|
@samtstern It's working locally, will push and we should be good to go hopefully. |
Signed-off-by: Alex Saveau <[email protected]>
|
Merging. In the future we should see if we can get release tests running. |
|
@samtstern Totally agree with you, created facebook/facebook-android-sdk#494 to address this issue. |
@samtstern Please don't merge yet, I want to do a wee bit of testing with Facebook.