-
Notifications
You must be signed in to change notification settings - Fork 14
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
Adopt Log service and DataStoreService changes from Core 2.0 #77
Adopt Log service and DataStoreService changes from Core 2.0 #77
Conversation
- Make IdentityStorageService methods non-static and inject from constuctor - Rename IdentityStorageService to IdentityStorageManager
@@ -43,9 +44,9 @@ final class ECID { | |||
*/ | |||
ECID(final String ecidString) { | |||
if (StringUtils.isNullOrEmpty(ecidString)) { | |||
MobileCore.log( | |||
LoggingMode.DEBUG, | |||
Log.debug( | |||
IdentityConstants.LOG_TAG, |
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.
Let's standardize usage of logs.
Few files have import static com.adobe.marketing.mobile.edge.identity.IdentityConstants.LOG_TAG;
I prefer IdentityConstants.LOG_TAG
> import com.adobe.marketing.mobile.edge.identity.IdentityConstants.LOG_TAG
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.
Not sure why this occurrence has a qualified import 😕 (because I did a replace all)!
I don't mind either way, but if I had to choose I would be inclined towards the static import as it is less verbose (especially when used a lot of time in the class) and also because it is an unambiguous import.
code/edgeidentity/src/main/java/com/adobe/marketing/mobile/edge/identity/IdentityExtension.java
Outdated
Show resolved
Hide resolved
...eidentity/src/main/java/com/adobe/marketing/mobile/edge/identity/IdentityStorageManager.java
Outdated
Show resolved
Hide resolved
code/edgeidentity/src/main/java/com/adobe/marketing/mobile/edge/identity/IdentityState.java
Show resolved
Hide resolved
...eidentity/src/main/java/com/adobe/marketing/mobile/edge/identity/IdentityStorageManager.java
Show resolved
Hide resolved
...eidentity/src/main/java/com/adobe/marketing/mobile/edge/identity/IdentityStorageManager.java
Outdated
Show resolved
Hide resolved
...eidentity/src/main/java/com/adobe/marketing/mobile/edge/identity/IdentityStorageManager.java
Outdated
Show resolved
Hide resolved
code/edgeidentity/src/main/java/com/adobe/marketing/mobile/edge/identity/IdentityExtension.java
Outdated
Show resolved
Hide resolved
code/edgeidentity/src/main/java/com/adobe/marketing/mobile/edge/identity/IdentityState.java
Show resolved
Hide resolved
code/edgeidentity/src/main/java/com/adobe/marketing/mobile/edge/identity/IdentityState.java
Show resolved
Hide resolved
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.
Looks good. Left a question for the test coverage, but feel free to take that in the tests PR
Description
Core 2.0 now provides a Log service and a DataStoreService. Use these to replace existing logging and storage management logic.
Related Issue
MOB-17069
Motivation and Context
Core 2.0 now provides a Log service and a DataStoreService. Use these to replace existing logging and storage management logic.
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: