[ios] Add SDK version update checking/notification #8282

friedbunny · 2017-03-05T04:47:40Z

Implements #7311 for iOS. This adds a check that the developer is using the latest stable release of our iOS SDK and, if not, prints a note to console with a link to the new release.

Inspired by Realm’s update check. 🙇

Only runs if using the simulator — method is a no-op anywhere else, including Swift Playgrounds.
Version comparison is extremely simple and ignores semver — if the version strings don’t match, it assumes the developer needs to upgrade.
Deploy script uploads a text file to http://mapbox.s3.amazonaws.com/mapbox-gl-native/ios/latest_version with the version in format: x.x.x
- @mick Is s3 the best location for this link? Would it be better on https://mapbox.com/ios-sdk?
Adds a SDK_UPDATE_AVAILABLE string that can be localized.

/cc @incanus @tobrun

mention-bot · 2017-03-05T04:47:44Z

@friedbunny, thanks for your PR! By analyzing this pull request, we identified @1ec5, @incanus and @boundsj to be potential reviewers.

1ec5 · 2017-03-05T19:47:19Z

platform/ios/resources/ja.lproj/Localizable.strings

@@ -43,6 +43,9 @@
 /* Action sheet title */
 "SDK_NAME" = "Mapbox iOS SDK";

+/* Developer-only SDK update notification; {latest version, in format x.x.x} */
+"SDK_UPDATE_AVAILABLE" = "現在Mapbox iOS SDK %1$@が入手できる：";


In general, you should wait until Transifex notices the new base string to translate it over there, rather than checking the translation in ahead of time.

I added this as a way to validate my localization implementation — should we take it out?

I'm not sure. I guess we can leave it in and take it out if there's any fallout.

1ec5 · 2017-03-05T19:50:27Z

platform/ios/src/MGLMapView.mm

@@ -564,6 +565,8 @@ - (void)commonInit
    if ([UIApplication sharedApplication].applicationState != UIApplicationStateBackground) {
        [MGLMapboxEvents pushEvent:MGLEventTypeMapLoad withAttributes:@{}];
    }
+
+    [MGLSDKUpdateChecker checkForUpdates];


Have you considered implementing this check in a +load method?

I also like the idea of calling this from the load or initialize class methods. If you do move it, calling from initialize like Realm do may be slightly safer since this ends up making a network call but probably fine either way.

+load in MGLMapView or MGLSDKUpdateChecker?

(Responded initially without refreshing and seeing @boundsj comments.)

Sure, we can move this to a new +load or +initialize in MGLMapView. This update check is clearly not essential, so that’s why I dropped it at the end of -commonInit.

Moved this call to +[MGLMapView initialize] in 4d3f390.

+[MGLOfflineStorage initialize] or -[MGLOfflineStorage init] would also cover the case where the developer uses offline packs early on but buries the map view in a secondary screen. But that's an edge case, so I'm fine with putting it here. Basically we want this check to happen early on while the developer is still paying attention.

Fine with putting it in +[MGLMapView initialize], that is.

1ec5 · 2017-03-05T19:51:29Z

platform/ios/src/MGLSDKUpdateChecker.mm

+ (void)checkForUpdates {
+#if TARGET_IPHONE_SIMULATOR
+    // Abort if running in a playground.
+    if ([[NSBundle mainBundle].bundleIdentifier hasPrefix:@"com.apple.dt.playground."]) {


Does this run as part of the Interface Builder designable agent process? You can use -[NSProcessInfo(MGLAdditions) mgl_isInterfaceBuilderDesignablesAgent] to check for that condition.

Will investigate this — likely no harm in adding such a guard, anyway.

Did not see evidence of the update check being called in IB, but IB is also failing to load our designable anyway... so I added the process exclusion in 470c6c0.

I later realized we bail in IB at the beginning of -commonInit. But this is a good check to add now that we're running code in the initialize method.

boundsj

This is great! I agree with @1ec5's that we should investigate moving the call and try to make sure this check does not happen because of IB. It'd be great to land this for the next 3.5.0 release later this week.

boundsj · 2017-03-06T18:29:38Z

platform/ios/src/MGLMapView.mm

@@ -564,6 +565,8 @@ - (void)commonInit
    if ([UIApplication sharedApplication].applicationState != UIApplicationStateBackground) {
        [MGLMapboxEvents pushEvent:MGLEventTypeMapLoad withAttributes:@{}];
    }
+
+    [MGLSDKUpdateChecker checkForUpdates];


I also like the idea of calling this from the load or initialize class methods. If you do move it, calling from initialize like Realm do may be slightly safer since this ends up making a network call but probably fine either way.

mick · 2017-03-06T19:17:23Z

@mick Is s3 the best location for this link? Would it be better on https://mapbox.com/ios-sdk?

👍 to the link being to s3

friedbunny · 2017-03-07T17:46:27Z

I think we’re ready to go here. Should this have a changelog entry?

1ec5 · 2017-03-07T18:28:01Z

Yes, I think it should have a changelog entry.

incanus · 2017-03-07T19:54:16Z

This looks great.

friedbunny · 2017-03-19T05:11:53Z

Moving the responsibility for updating and hosting the checker file to the ios-sdk site in #8468.

friedbunny added build feature iOS Mapbox Maps SDK for iOS labels Mar 5, 2017

friedbunny added this to the ios-v3.5.0 milestone Mar 5, 2017

friedbunny self-assigned this Mar 5, 2017

friedbunny requested review from boundsj and 1ec5 March 5, 2017 04:47

1ec5 reviewed Mar 5, 2017

View reviewed changes

boundsj reviewed Mar 6, 2017

View reviewed changes

boundsj approved these changes Mar 6, 2017

View reviewed changes

friedbunny added 2 commits March 7, 2017 10:43

[ios] Add SDK version update checking/notification

5e36743

[ios] Add version checker update to deploy script

a6bbf09

friedbunny force-pushed the fb-update-checker branch from 4d3f390 to a6bbf09 Compare March 7, 2017 18:47

friedbunny merged commit a6bbf09 into release-ios-v3.5.0-android-v5.0.0 Mar 7, 2017

friedbunny deleted the fb-update-checker branch March 7, 2017 19:50

friedbunny mentioned this pull request Mar 10, 2017

consider dev update notification in IDE #7311

Closed

friedbunny mentioned this pull request Mar 19, 2017

[ios, build] Change version checker URL to ios-sdk site #8468

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ios] Add SDK version update checking/notification #8282

[ios] Add SDK version update checking/notification #8282

friedbunny commented Mar 5, 2017 •

edited

Loading

mention-bot commented Mar 5, 2017

1ec5 Mar 5, 2017

friedbunny Mar 6, 2017

1ec5 Mar 7, 2017

1ec5 Mar 5, 2017

boundsj Mar 6, 2017

friedbunny Mar 6, 2017

friedbunny Mar 6, 2017

friedbunny Mar 6, 2017

1ec5 Mar 7, 2017

1ec5 Mar 7, 2017

1ec5 Mar 5, 2017

friedbunny Mar 6, 2017

friedbunny Mar 6, 2017

1ec5 Mar 7, 2017

boundsj left a comment

boundsj Mar 6, 2017

mick commented Mar 6, 2017

friedbunny commented Mar 7, 2017

1ec5 commented Mar 7, 2017

incanus commented Mar 7, 2017

friedbunny commented Mar 19, 2017 •

edited

Loading

[ios] Add SDK version update checking/notification #8282

[ios] Add SDK version update checking/notification #8282

Conversation

friedbunny commented Mar 5, 2017 • edited Loading

mention-bot commented Mar 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boundsj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mick commented Mar 6, 2017

friedbunny commented Mar 7, 2017

1ec5 commented Mar 7, 2017

incanus commented Mar 7, 2017

friedbunny commented Mar 19, 2017 • edited Loading

friedbunny commented Mar 5, 2017 •

edited

Loading

friedbunny commented Mar 19, 2017 •

edited

Loading