Extension framework #427
Extension framework #427
Conversation
• The iOS clang linker will give a warning if an iOS app extension uses a framework that uses UIKit in some way • To fix this (and to improve separation of concerns), we will be splitting out Notification Service Extension functionality into its own separate framework
• Merges test reliability improvements from master into this branch to fix flaky tests so we can create a PR.
There was a problem hiding this comment.
Reviewed 17 of 27 files at r1.
Reviewable status: 17 of 27 files reviewed, 10 unresolved discussions (waiting on @Nightsd01 and @jkasten2)
iOS_SDK/OneSignalSDK/OneSignalExtension/NSURLSession+OneSignal.h, line 6 at r1 (raw file):
// // Created by Brad Hesse on 9/26/18. // Copyright © 2018 Hiptic. All rights reserved.
Updated header
iOS_SDK/OneSignalSDK/OneSignalExtension/NSURLSession+OneSignal.m, line 6 at r1 (raw file):
// // Created by Brad Hesse on 9/26/18. // Copyright © 2018 Hiptic. All rights reserved.
Update header
iOS_SDK/OneSignalSDK/OneSignalExtension/OneSignalExtension.h, line 6 at r1 (raw file):
// // Created by Brad Hesse on 9/26/18. // Copyright © 2018 Hiptic. All rights reserved.
Update header
iOS_SDK/OneSignalSDK/OneSignalExtension/OneSignalExtension.m, line 6 at r1 (raw file):
// // Created by Brad Hesse on 9/26/18. // Copyright © 2018 Hiptic. All rights reserved.
Updated file header
iOS_SDK/OneSignalSDK/Source/NSDictionary+OneSignal.h, line 6 at r1 (raw file):
// // Created by Brad Hesse on 9/26/18. // Copyright © 2018 Hiptic. All rights reserved.
Update header
iOS_SDK/OneSignalSDK/Source/NSDictionary+OneSignal.h, line 14 at r1 (raw file):
@interface NSDictionary (OneSignal) - (BOOL)isOneSignalPayload;
An Objective-C Category on NSDictionary seems to specific for this. Can we move this into a shared helper class instead?
iOS_SDK/OneSignalSDK/Source/OneSignalAttachmentsController.h, line 6 at r1 (raw file):
// // Created by Brad Hesse on 9/26/18. // Copyright © 2018 Hiptic. All rights reserved.
Update header
iOS_SDK/OneSignalSDK/Source/OneSignalAttachmentsController.m, line 6 at r1 (raw file):
// // Created by Brad Hesse on 9/26/18. // Copyright © 2018 Hiptic. All rights reserved.
Update header
iOS_SDK/OneSignalSDK/Source/OneSignalExtensionBadgeHandler.m, line 46 at r1 (raw file):
} int currentValue = (int)OneSignalExtensionBadgeHandler.currentCachedBadgeValue ?: 0;
Why change these from inferred types? If this is due to the pre processors missing can we moved them into a header that can be shared?
iOS_SDK/OneSignalSDK/Source/OneSignalShared.h, line 6 at r1 (raw file):
// // Created by Brad Hesse on 9/26/18. // Copyright © 2018 Hiptic. All rights reserved.
Update header
• Fixes header copyrights, forgot to modify these when adding new files in the previous commit.
• Decided to move the auto-type deduction macros from OneSignalHelper.h to OneSignalShared.h since it is being used now by both the main SDK and the Extension framework • Fixed some more header copyright mistakes
|
iOS_SDK/OneSignalSDK/Source/OneSignalExtensionBadgeHandler.m, line 46 at r1 (raw file): Previously, jkasten2 (Josh Kasten) wrote…
Fixed. Discussed this with Josh and decided it's a good idea to keep using let/var auto deduced type macros. Moved the auto-deduce type macros to |
There was a problem hiding this comment.
Reviewable status: 14 of 30 files reviewed, 10 unresolved discussions (waiting on @jkasten2 and @Nightsd01)
iOS_SDK/OneSignalSDK/OneSignalExtension/NSURLSession+OneSignal.h, line 6 at r1 (raw file):
Previously, jkasten2 (Josh Kasten) wrote…
Updated header
Done.
iOS_SDK/OneSignalSDK/OneSignalExtension/NSURLSession+OneSignal.m, line 6 at r1 (raw file):
Previously, jkasten2 (Josh Kasten) wrote…
Update header
Done.
iOS_SDK/OneSignalSDK/OneSignalExtension/OneSignalExtension.h, line 6 at r1 (raw file):
Previously, jkasten2 (Josh Kasten) wrote…
Update header
Done.
iOS_SDK/OneSignalSDK/OneSignalExtension/OneSignalExtension.m, line 6 at r1 (raw file):
Previously, jkasten2 (Josh Kasten) wrote…
Updated file header
Done.
iOS_SDK/OneSignalSDK/Source/NSDictionary+OneSignal.h, line 6 at r1 (raw file):
Previously, jkasten2 (Josh Kasten) wrote…
Update header
Done.
iOS_SDK/OneSignalSDK/Source/OneSignalAttachmentsController.h, line 6 at r1 (raw file):
Previously, jkasten2 (Josh Kasten) wrote…
Update header
Done.
iOS_SDK/OneSignalSDK/Source/OneSignalAttachmentsController.m, line 6 at r1 (raw file):
Previously, jkasten2 (Josh Kasten) wrote…
Update header
Done.
iOS_SDK/OneSignalSDK/Source/OneSignalExtensionBadgeHandler.m, line 46 at r1 (raw file):
Previously, Nightsd01 (Brad Hesse) wrote…
Fixed. Discussed this with Josh and decided it's a good idea to keep using let/var auto deduced type macros. Moved the auto-deduce type macros to
OneSignalShared.hso that it can be shared between the SDK and Extension framework.
Done.
iOS_SDK/OneSignalSDK/Source/OneSignalShared.h, line 6 at r1 (raw file):
Previously, jkasten2 (Josh Kasten) wrote…
Update header
Done.
There was a problem hiding this comment.
Reviewed 2 of 27 files at r1, 15 of 15 files at r2.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @jkasten2)
iOS_SDK/OneSignalSDK/Source/NSDictionary+OneSignal.h, line 14 at r1 (raw file):
Previously, jkasten2 (Josh Kasten) wrote…
An Objective-C Category on NSDictionary seems to specific for this. Can we move this into a shared helper class instead?
Talked about this in person, there isn't a good spot to move this to and this makes it's usage much cleaner where it is used. So I am ok keeping it like this.
There was a problem hiding this comment.
Reviewable status:
complete! all files reviewed, all discussions resolved
• The iOS clang linker will emit a warning if an iOS app extension uses a framework that uses UIKit (or some other extension-unavailable API)
• To fix this (and to improve separation of concerns), we will be splitting out Notification Service Extension functionality into its own separate framework for the next major release (3.0)
This change is