-
Notifications
You must be signed in to change notification settings - Fork 112
Merge and small changes #839
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
…-sdk # Conflicts: # Examples/restcomm-olympus/app/app.iml
params.put(RCDevice.ParameterKeys.PUSH_NOTIFICATIONS_ENABLE_PUSH_FOR_ACCOUNT, prefs.getBoolean(RCDevice.ParameterKeys.PUSH_NOTIFICATIONS_ENABLE_PUSH_FOR_ACCOUNT , true)); | ||
params.put(RCDevice.ParameterKeys.PUSH_NOTIFICATIONS_PUSH_DOMAIN, prefs.getString(RCDevice.ParameterKeys.PUSH_NOTIFICATIONS_PUSH_DOMAIN ,"")); | ||
params.put(RCDevice.ParameterKeys.PUSH_NOTIFICATIONS_HTTP_DOMAIN, prefs.getString(RCDevice.ParameterKeys.PUSH_NOTIFICATIONS_HTTP_DOMAIN ,"")); | ||
params.put(RCDevice.ParameterKeys.PUSH_NOTIFICATIONS_FCM_SERVER_KEY, prefs.getString(RCDevice.ParameterKeys.PUSH_NOTIFICATIONS_FCM_SERVER_KEY ,"")); |
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'd say let's continue retrieving the value from preferences, but update SigninActivity with BuildConfig. TEST_PUSH_FCM_KEY to make it cleaner and more consistent. Otherwise we run the risk of trying to update prefs in signin activity and having no effect
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.
Sure
// Not specifying applicationIdSuffix since release uses the default applicationId: org.restcomm.android.olympus | ||
buildConfigField "boolean", "ENABLE_TEST_FAIRY_RUNTIME", "true" | ||
buildConfigField "String", "TESTFAIRY_APP_TOKEN", TESTFAIRY_APP_TOKEN | ||
buildConfigField "String", "TEST_PUSH_FCM_KEY", TEST_PUSH_FCM_KEY |
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 think we should also put this in debug build configuration so that we can use push and test with push on any build configuration.
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.
Its already there. I just added this to release. Or did you think something else?
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.
Whoops, missed it; cool
// Not specifying applicationIdSuffix since release uses the default applicationId: org.restcomm.android.olympus | ||
buildConfigField "boolean", "ENABLE_TEST_FAIRY_RUNTIME", "true" | ||
buildConfigField "String", "TESTFAIRY_APP_TOKEN", TESTFAIRY_APP_TOKEN | ||
buildConfigField "String", "TEST_PUSH_FCM_KEY", TEST_PUSH_FCM_KEY |
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.
Now I see why you wanted to take the TEST_ out :). But since we 'll be going over productization soon we probably will separate test and production values. So let's leave it as is for now
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.
ok:)
No description provided.