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

[ios, macos] Expose the url session configuration object. #13886

Merged
merged 5 commits into from
Feb 18, 2019
Merged

[ios, macos] Expose the url session configuration object. #13886

merged 5 commits into from
Feb 18, 2019

Conversation

fabian-guerra
Copy link
Contributor

Fixes #12026

@fabian-guerra fabian-guerra added feature iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS needs changelog Indicates PR needs a changelog entry prior to merging. labels Feb 7, 2019
@fabian-guerra fabian-guerra added this to the release-kombucha milestone Feb 7, 2019
@fabian-guerra fabian-guerra self-assigned this Feb 7, 2019
@fabian-guerra fabian-guerra requested a review from a team February 7, 2019 00:26
@fabian-guerra fabian-guerra force-pushed the fabian-sessionconfig-12026 branch 3 times, most recently from c483816 to 3ae974c Compare February 7, 2019 23:16
@friedbunny
Copy link
Contributor

Given that we’re considering removing MGLNetworkConfiguration (#12417) and @jmkiley has started doing so in #13754, the two of you may wish to coordinate.

@jmkiley jmkiley mentioned this pull request Feb 11, 2019
Merged
fabian-guerra
fabian-guerra commented Feb 12, 2019
View reviewed changes
platform/darwin/src/MGLNetworkConfiguration.m Outdated
});
};
if (![[NSThread currentThread] isMainThread]) {
dispatch_sync(dispatch_get_main_queue(), ^{
// This is a temporary fix to avoid a dead lock
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be removed once this #13754 lands.

julianrex
julianrex approved these changes Feb 18, 2019
View reviewed changes
Copy link
Contributor

@julianrex julianrex left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!

platform/darwin/src/MGLNetworkConfiguration.h Outdated
will not affect the behavior of that session.

*/
@property (strong, null_resettable) NSURLSessionConfiguration *sessionConfiguration;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you're using @synchronized below, what do you think to marking this as atomic? (essentially documentation)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I am. @atomic is the default property if @nonatomic is not present. I can add it to make it clear tho.

@fabian-guerra fabian-guerra merged commit 1fc7a9b into master Feb 18, 2019
@fabian-guerra fabian-guerra deleted the fabian-sessionconfig-12026 branch February 18, 2019 21:16
@halset halset mentioned this pull request Feb 18, 2019
Closed
@sebastianludwig
Copy link

Awesome! Thank you very much!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@julianrex julianrex julianrex approved these changes

@friedbunny friedbunny Awaiting requested review from friedbunny

@1ec5 1ec5 Awaiting requested review from 1ec5

Assignees

@fabian-guerra fabian-guerra

Labels
feature iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS needs changelog Indicates PR needs a changelog entry prior to merging.
Projects
None yet
Milestone
release-kombucha
Development

Successfully merging this pull request may close these issues.
4 participants
@fabian-guerra @friedbunny @sebastianludwig @julianrex