iziz / libPhoneNumber-iOS

iOS port from libphonenumber (Google's phone number handling library)
Apache License 2.0
2.35k stars 459 forks source link

Fixed 'subscriberCellularProvider' is deprecated: first deprecated in… #365

Closed FONickReichard closed 3 months ago

FONickReichard commented 2 years ago

Fixed iOS 12.0 warning: 'subscriberCellularProvider' is deprecated: first deprecated in iOS 12.0 Replace 'subscriberCellularProvider' with 'serviceSubscriberCellularProviders'

Edit 04/06/22: Does not resolve warning, however prevents depreciation for iOS 16.

Edit2 04/06/22: PR feedback resolves warning.

LowAmmo commented 2 years ago

Does this actually alleviate the warning in the code? or is this just defensive coding incase that function totally goes away in iOS 16?

FONickReichard commented 2 years ago

Good question. It does not alleviate the warning in code. It's a defense against iOS 16. I didn't want to remove the subscriberCellularProvider since it's iOS 4.0 - 12.0, and the serviceSubscriberCellularProviders is for 12.0 +.

LowAmmo commented 2 years ago

I don't know who has the authority to make the decision...but...I would propose we just set iOS 12 as the minimum supported OS version (more than 3 years old at this point)...and don't even need the if #available check....

Seems like even setting the minimum supported version to iOS 12 is a little dated...

https://www.statista.com/statistics/565270/apple-devices-ios-version-share-worldwide/

FONickReichard commented 2 years ago

Agreed. Hopefully there aren't too many projects using this that are running below iOS 12. Definitely over due to bump up the minimum supported iOS.

LowAmmo commented 2 years ago

Also, I guess another option would be something like this -

 - (NSString *)countryCodeByCarrier {
 #if __IPHONE_OS_VERSION_MIN_REQUIRED >= 120000
     NSDictionary<NSString *, CTCarrier *> *serviceSubscriberCellularProviders = [self.telephonyNetworkInfo serviceSubscriberCellularProviders];
     CTCarrier *carrier = serviceSubscriberCellularProviders.allValues.firstObject;

     if (nil==carrier) {
         return NB_UNKNOWN_REGION;
     }

     return carrier.isoCountryCode;
 #else
     NSString *isoCode = [[self.telephonyNetworkInfo subscriberCellularProvider] isoCountryCode];

     // The 2nd part of the if is working around an iOS 7 bug
     // If the SIM card is missing, iOS 7 returns an empty string instead of nil
     if (isoCode.length == 0) {
       isoCode = NB_UNKNOWN_REGION;
     }

     return isoCode;
 #endif

If someone got upset about upping the minimum supported to iOS 12 (if someone does get upset...I'd be REALLY curious on the reasoning...)

FONickReichard commented 2 years ago

Thanks @LowAmmo!

LowAmmo commented 2 years ago

Thank you, @FONickReichard for taking the initiative!

Now just need to get his merged and get an updated cocoapod release out the door... 😄

paween commented 2 years ago

Actually, if you want to set minimum iOS version to 12, I think it's fine to do that. Please send me a PR and I'll approve it.

FONickReichard commented 2 years ago

@paween Sounds good. I'll do a separate PR. Is it all good on your end if I update the project settings to be built with Xcode 13.0 (13A233) recommend settings?

https://developer.apple.com/news/?id=2t1chhp3

paween commented 2 years ago

Sure I think that’s fine too.

On Apr 6, 2022, at 4:43 PM, FONickReichard @.***> wrote:



@paweenhttps://github.com/paween Sounds good. I'll do a separate PR. Is it all good on your end if I update the project settings to be built with Xcode 13.0 (13A233) recommend settings?

https://developer.apple.com/news/?id=2t1chhp3

— Reply to this email directly, view it on GitHubhttps://github.com/iziz/libPhoneNumber-iOS/pull/365#issuecomment-1090930853, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AADBJ6POAZG5GC5ZMVNM3ADVDYORLANCNFSM5SUCSH5A. You are receiving this because you were mentioned.Message ID: @.***>

LowAmmo commented 1 year ago

@paween - Any chance of getting this merged? (along with maybe doing an updated release to cocoapods??)

-Thanks!

LowAmmo commented 1 year ago

Would love to get this merged to master, and released, and an updated version uploaded/updated to Cocoapods... 😄

FONickReichard commented 1 year ago

@paiv Any thing I can do to help get this merged?