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.

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

FirebaseMessaging: NSUserDefaults replaced by GULUserDefaults #2443

Merged
merged 3 commits into from
Feb 26, 2019
Merged

FirebaseMessaging: NSUserDefaults replaced by GULUserDefaults #2443

merged 3 commits into from
Feb 26, 2019

Conversation

maksymmalyhin
Copy link
Contributor

This is a fix for #1919:

  • NSUserDefaults replaced by GULUserDefaults in FirebaseMessaging

@maksymmalyhin maksymmalyhin self-assigned this Feb 25, 2019
ryanwilson
ryanwilson reviewed Feb 25, 2019
View reviewed changes
Copy link
Member

@ryanwilson ryanwilson left a comment

Choose a reason for hiding this comment

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

You'll also need to add it as a dependency in the Podspec:

firebase-ios-sdk/FirebaseMessaging.podspec

Line 44 in 7cc0a9b

s.dependency 'GoogleUtilities/Environment', '~> 5.2'

Firebase/Messaging/FIRMessagingPubSub.m Outdated
@@ -23,6 +23,7 @@
#import "FIRMessagingPendingTopicsList.h"
#import "FIRMessagingUtilities.h"
#import "FIRMessaging_Private.h"
#import <GoogleUtilities/GULUserDefaults.h>
Copy link
Member

Choose a reason for hiding this comment

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

Nit: we use a different import style, it should be handled if you run the style script with ./scripts/style.sh in the root of the repo.

@maksymmalyhin
Copy link
Contributor Author

You'll also need to add it as a dependency in the Podspec:

firebase-ios-sdk/FirebaseMessaging.podspec

Line 44 in 7cc0a9b

s.dependency 'GoogleUtilities/Environment', '~> 5.2'

@ryanwilson Should I use the same version as for other GoogleUtilities - '~> 5.2'?

@ryanwilson
Copy link
Member

@maksymmalyhin great question, looks like we'll need to update to 5.3 since that's when the UserDefaults subspec was introduced (looking at https://github.com/firebase/firebase-ios-sdk/commits/master/GoogleUtilities.podspec)

paulb777
paulb777 approved these changes Feb 25, 2019
View reviewed changes
Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

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

LGTM, good to merge on @chliangGoogle's approval.

@maksymmalyhin maksymmalyhin merged commit 8ffd959 into master Feb 26, 2019
@maksymmalyhin maksymmalyhin deleted the mm-msg-userdefaults branch February 27, 2019 02:43
@firebase firebase locked and limited conversation to collaborators Oct 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ryanwilson ryanwilson ryanwilson left review comments

@paulb777 paulb777 paulb777 approved these changes

@charlotteliang charlotteliang charlotteliang approved these changes

Assignees

@maksymmalyhin maksymmalyhin

Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@maksymmalyhin @ryanwilson @paulb777 @charlotteliang @googlebot