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

Add new remote params #1076

Merged
merged 4 commits into from
Aug 4, 2020
Merged

Add new remote params #1076

merged 4 commits into from
Aug 4, 2020

Conversation

Jeasmine
Copy link
Contributor

@Jeasmine Jeasmine commented Jun 25, 2020

  • Add unsubscribeWhenNotificationAreDisable remote param
  • Add disableGMSMissingPrompt remote param
  • Add locationShared remote param
  • Add requiresUserPrivacyConcent remote param
  • Keep public method
  • Add tests for new getters

This change is Reviewable

@Jeasmine Jeasmine force-pushed the feature/remote-params branch 4 times, most recently from 54cfba8 to 130a023 Compare July 6, 2020 14:59
Copy link
Contributor

@mikechoch mikechoch left a comment

Choose a reason for hiding this comment

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

I think this branch should be rebased to major_release_4.0.0, looks like we are avoiding breaking changes in this PR but there should be breaking changes because we are removing specific variables and methods and replacing them with the remote param methods and defaults.

@Jeasmine Jeasmine changed the base branch from master to major_release_4.0.0 July 14, 2020 18:57
   * Add unsubscribeWhenNotificationAreDisable remote param
   * Add disableGMSMissingPrompt remote param
   * Add locationShared remote param
   * Add requiresUserPrivacyConcent remote param
   * Keep public method
   * Add tests for new getters
   * Move task management to TaskController
   * Delay task until remote params and init is done
   * After task delayed, check again for user consent
   * Fix test, add tests
@Jeasmine Jeasmine force-pushed the feature/remote-params branch 2 times, most recently from ac30cb8 to b43e72b Compare August 3, 2020 18:26
Copy link
Member

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

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

Just one nit on an extra if statement.

Copy link
Contributor

@mikechoch mikechoch left a comment

Choose a reason for hiding this comment

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

LGTM

@Jeasmine Jeasmine merged commit a30020e into major_release_4.0.0 Aug 4, 2020
@Jeasmine Jeasmine deleted the feature/remote-params branch August 4, 2020 18:03
mikechoch pushed a commit that referenced this pull request Aug 13, 2020
* Add new remote params

   * Add unsubscribeWhenNotificationAreDisable remote param
   * Add disableGMSMissingPrompt remote param
   * Add locationShared remote param
   * Add requiresUserPrivacyConcent remote param
   * Keep public method
   * Add tests for new getters

* Add task delay due to requiring consent from remote params

   * Move task management to TaskController
   * Delay task until remote params and init is done
   * After task delayed, check again for user consent
   * Fix test, add tests

* Remove extra check/schedule on pending tasks
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