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

[ios, macos] Mark deprecated methods as unavailable #11205

Merged
merged 12 commits into from
Feb 17, 2018

Conversation

jmkiley
Copy link
Contributor

@jmkiley jmkiley commented Feb 14, 2018

Fixes #10735

  • Marked methods/properties as unavailable.
  • Removed the implementation code.
  • Remove related tests.
  • Clean up comments.

@jmkiley jmkiley added iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS SEMVER-MAJOR Requires a major release according to Semantic Versioning rules in progress labels Feb 14, 2018
@jmkiley jmkiley added this to the ios-v4.0.0 milestone Feb 14, 2018
@jmkiley jmkiley self-assigned this Feb 14, 2018
@jmkiley jmkiley requested review from friedbunny and 1ec5 February 14, 2018 18:25
@@ -55,7 +55,7 @@ + (instancetype)sharedManager {
}

+ (BOOL)mapboxMetricsEnabledSettingShownInApp {
NSLog(@"mapboxMetricsEnabledSettingShownInApp is no longer necessary; the Mapbox Maps SDK for iOS has changed to always provide a telemetry setting in-app.");
[NSException raise:NSInternalInconsistencyException format:@"Telemetry settings are now always shown in the ℹ️ menu."];
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you say more about why we shouldn’t delete this implementation?


@deprecated Call the relevant class method of `MGLStyle` for the URL of a
particular default style.
:nodoc: bundledStyleURLs has been removed.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it’s safe to get rid of any external comments on unavailable() bits.

@@ -284,16 +281,16 @@ MGL_EXPORT IB_DESIGNABLE
- (IBAction)showAttribution:(id)sender;

/// :nodoc: Support for style classes has been removed. This property always returns an empty array.
Copy link
Contributor

Choose a reason for hiding this comment

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

These comments are no longer accurate — let’s axe ‘em.

@@ -89,7 +89,7 @@ MGL_EXPORT

Emerald is a tactile style with subtle textures and dramatic hillshading.
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto to removing these obsolete comments.

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.

Let’s add a blurb to both changelogs referring to the removal of these symbols. Something like:

Removed methods, properties, and constants that had been deprecated as of v3.7.x.

return MGLStyleURL_hybrid;
}

// Emerald is no longer getting new versions as a default style, so the current version is hard-coded here.
Copy link
Contributor

Choose a reason for hiding this comment

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

Goodbye Emerald. 😢

@1ec5
Copy link
Contributor

1ec5 commented Feb 15, 2018

1096388 removes Traffic Day and Traffic Night menu items from macosapp.

@jmkiley
Copy link
Contributor Author

jmkiley commented Feb 15, 2018

I am currently working on getting a couple of tests passing. I'm not sure if I should remove the trafficDay and trafficNight styles from default_styles.hpp, since other platforms still seem to use those styles. Another option is to modify the tests so they pass until we add the navigation styles or remove the unavailable styles from core.

@@ -100,15 +100,6 @@ - (void)testVersionedStyleURLs {
XCTAssertEqualObjects([MGLStyle satelliteStreetsStyleURLWithVersion:99].absoluteString,
@"mapbox://styles/mapbox/satellite-streets-v99");
#pragma clang diagnostic push
Copy link
Contributor

@friedbunny friedbunny Feb 15, 2018

Choose a reason for hiding this comment

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

The pushes and pops can be removed, too — they served to limit the scope of the ignored warnings.

@1ec5
Copy link
Contributor

1ec5 commented Feb 15, 2018

I am currently working on getting a couple of tests passing. I'm not sure if I should remove the trafficDay and trafficNight styles from default_styles.hpp, since other platforms still seem to use those styles. Another option is to modify the tests so they pass until we add the navigation styles or remove the unavailable styles from core.

mbgl::util::default_styles::orderedStyles is used by the GLFW application and the Qt SDK. I think it’d be reasonable to remove them from orderedStyles. After all, they’ve been superseded by the Navigation Guidance Day/Night v2 styles, even if the latter styles aren’t currently listed.

/cc @mapbox/qt

@jmkiley
Copy link
Contributor Author

jmkiley commented Feb 16, 2018

Without the declarations in default_styles.hpp, the tests fail here. Currently, the regex does not check whether styleURLs are unavailable. I can update that test tomorrow to filter out the unavailable styles.

@jmkiley
Copy link
Contributor Author

jmkiley commented Feb 16, 2018

My current solution is to check whether a style's name contains traffic. I wasn't able to get an approach that checks for the unavailable flag working.

@jmkiley jmkiley merged commit 9459cf6 into release-boba Feb 17, 2018
@jmkiley jmkiley deleted the jmkiley-mark-unavailable branch February 17, 2018 00:20
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 SEMVER-MAJOR Requires a major release according to Semantic Versioning rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants