Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[ios, macos] Convert various class methods to class properties #11674

Merged
merged 1 commit into from
Apr 16, 2018

Conversation

friedbunny
Copy link
Contributor

@friedbunny friedbunny commented Apr 12, 2018

Fixes #6778. This improves our Swift interface in an unavoidably breaking way while mostly having no effect on Obj-C usage.

For example, before → after:

  • MGLStyle.streetsStyleURL()MGLStyle.streetsStyleURL
  • MGLOfflineStorage.shared()MGLOfflineStorage.shared
  • MGLAccountManager.setAccessToken("foo")MGLAccountManager.accessToken = "foo”

This also updates a variety of internal/undocumented classes in a similar way — this doesn’t really matter while we’re using Obj-C, but future-proofs potential Swiftification.

To-do (suggestions welcome)

  • Update generated darwin examples.
  • Improve changelog entry.
  • Audit my choices in property values — e.g., nonatomic?
  • Double check SDK docs here for missed updates.

/cc @mapbox/maps-ios

@friedbunny friedbunny added iOS Mapbox Maps SDK for iOS refactor macOS Mapbox Maps SDK for macOS SEMVER-MAJOR Requires a major release according to Semantic Versioning rules labels Apr 12, 2018
@friedbunny friedbunny added this to the ios-v4.0.0 milestone Apr 12, 2018
@friedbunny friedbunny self-assigned this Apr 12, 2018
@@ -3,7 +3,7 @@
@interface MGLAccountManager (Private)

/// Returns the shared instance of the `MGLAccountManager` class.
+ (instancetype)sharedManager;
@property (class, nonatomic, readonly) MGLAccountManager *sharedManager;
Copy link
Contributor

Choose a reason for hiding this comment

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

The use of nonatomic for these class properties is consistent with previous behavior when these properties were class methods. atomic is a remnant of cargo-cult programming that resulted in excessive locking and dispatching early on.

@@ -11,6 +11,7 @@ The 4.0._x_ series of releases will be the last to support iOS 8. The minimum iO
* Removed support for 32-bit simulators. ([#10962](https://github.com/mapbox/mapbox-gl-native/pull/10962))
* Added Danish and Hebrew localizations. ([#10967](https://github.com/mapbox/mapbox-gl-native/pull/10967), [#11136](https://github.com/mapbox/mapbox-gl-native/pull/11134))
* Removed methods, properties, and constants that had been deprecated as of v3.7.4. ([#11205](https://github.com/mapbox/mapbox-gl-native/pull/11205))
* Refined certain Swift interfaces by converting them from class methods to class properties. ([#11674](https://github.com/mapbox/mapbox-gl-native/pull/11674))
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add the same blurb to the macOS changelog. Thanks!

@1ec5
Copy link
Contributor

1ec5 commented Apr 13, 2018

Some auxiliary content to update:

@friedbunny
Copy link
Contributor Author

This is ready for review. 🙋‍♂️

@friedbunny friedbunny added the Swift Specific to the Swift/Objective-C bridge on iOS or macOS label Apr 13, 2018
Copy link
Contributor

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

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

💯

This improves the Swift interface while having no effect on Obj-C usage.
@friedbunny friedbunny merged commit d4e597e into release-boba Apr 16, 2018
@friedbunny friedbunny deleted the fb-objc-class-properties branch April 16, 2018 23:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS refactor SEMVER-MAJOR Requires a major release according to Semantic Versioning rules Swift Specific to the Swift/Objective-C bridge on iOS or macOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants