-
Notifications
You must be signed in to change notification settings - Fork 5
refactor: merge storages #99
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
base: master
Are you sure you want to change the base?
Conversation
OptimoveNotificationServiceExtension/Sources/OptimoveNotificationService.swift
Outdated
Show resolved
Hide resolved
OptimoveNotificationServiceExtension/Sources/OptimoveNotificationService.swift
Show resolved
Hide resolved
|
Can we change the version to 6.0 as part of this PR? Maybe makes sense to change the target branch. Should we add the Changelog as well? are there any docs we need to change in the wiki? |
I don't think we have breaking changes here for a major version. |
Won't a client targeting a lower then iOS version 13 be forced to upgrade? |
You're right. I'm going to change it asap. |
|
@CodiumAI-Agent /review |
PR Analysis
PR Feedback💡 General suggestions: The PR is well-structured and includes a comprehensive list of changes. The refactor appears to be well thought out and the addition of tests is commendable. However, it would be beneficial to ensure that all changes are thoroughly tested, especially given the scale of the refactor. It would also be beneficial to ensure that the changes are backward compatible where possible, or to provide clear migration paths where not. 🤖 Code feedback:
✨ Usage guide:Overview: With a configuration file, use the following template:
See the review usage page for a comprehensive guide on using this tool. |
PR Review
Code feedback:
✨ Review tool usage guide:Overview: With a configuration file, use the following template:
See the review usage page for a comprehensive guide on using this tool. |
…/notification-service-extension-2 # Conflicts: # CHANGELOG.md # OptimoveSDK/Sources/Classes/Optimobile/Optimobile+DeepLinking.swift
…/notification-service-extension-2 # Conflicts: # OptimoveSDK/Sources/Classes/Optimobile/Optimobile.swift
…/notification-service-extension-2 # Conflicts: # OptimoveSDK/Sources/Classes/DI/Assembly.swift # OptimoveSDK/Sources/Classes/Migration/MigrationWork.swift # OptimoveSDK/Sources/Classes/Migration/Version.swift # OptimoveSDK/Sources/Classes/Storage/OptimoveStorage.swift
|
👋 Hey @eligutovsky, ❗️ Work Item link check failed: There is no Work Item linked to this PR. 🧩 Please add the Work Item ID to the PR title or description using the "Edit" option in the format below: 🟢 Once added, the validation will automatically run again! |
User description
Description of Changes
This PR is a result of a series of changes #93, #91, #90, #89, #87
that contains:
Breaking Changes
Release Checklist
Prepare:
pod lib lintpassesBump versions in:
OptimoveCore.podspecOptimoveNotificationServiceExtension.podspecOptimoveSDK.podspecOptimoveCore/Sources/Classes/Constants/SDKVersion.swiftREADME.mdCHANGELOG.mdUpdate major version numbers in wiki (basic integration + push guides)
Integration tests
T&T Only
Mobile Only
Deferred Deep Links
Combined
Release:
pod trunk pushto publish to CocoaPodsPost Release:
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Refactor the SDK's data persistence by introducing a unified
KeyValueStoragesystem and migrating existing user data, ensuring seamless upgrades and consistent data access. Rewrite the Notification Service Extension to utilize modern SwiftCodablemodels andasync/awaitfor improved push notification handling, including badge management and media attachments. Modernize internal APIs and code structure by adjusting access controls and removing redundant components, aligning the SDK with iOS 13+ requirements.KeyValueStorageprotocol andStorageKeyenum, consolidatingUserDefaultsusage across standard and app group containers. This includes a significant migration (MigrationWork_6_0_0) to move existing data from the old KumulosUserDefaultskeys to the new OptimoveStorageKeysystem, ensuring seamless upgrades and consistent data access throughout the SDK.Modified files (40)
Latest Contributors(2)
publictointernal), removing redundant code, and updating various internal components to align with modern Swift practices and the new iOS 13+ minimum version. This includes changes to configuration models, network clients, and utility classes, enhancing encapsulation and preparing for future development.Modified files (62)
Latest Contributors(2)
Codableprotocol for robust payload parsing andasync/awaitfor asynchronous operations. This includes introducing newPushNotificationandMediaHelpermodels, updating badge management, and ensuring compatibility with iOS 13+ features, while also adding comprehensive unit tests for the new components.Modified files (31)
Latest Contributors(2)