[Returned for Revisions] SDL 0061 - Locale support

[theresalech]

Hello SDL community,

The review of the revised proposal "SDL 0061 - Locale support" begins now and runs through August 29, 2017. The original review of "Locale support" occurred May 10 through May 16, 2017. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0061-locale-support.md

Reviews are an important part of the SDL evolution process. All reviews should be sent to the associated Github issue at:

#179

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of SDL. When writing your review, here are some questions you might want to answer in your review:

• Is the problem being addressed significant enough to warrant a change to SDL?
• Does this proposal fit well with the feel and direction of SDL?
• If you have used competitors with a similar feature, how do you feel that this proposal compares to those?
• How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
  Please state explicitly whether you believe that the proposal should be accepted into SDL.

More information about the SDL evolution process is available at

https://github.com/smartdevicelink/sdl_evolution/blob/master/process.md

Thank you,
Theresa Lech

Program Manager - Livio
theresa@livio.io

[joeljfischer]

I think this proposal needs to give the proposed changes to the mobile_api in XML form so that we can more clearly see these changes. It's not obvious given the proposal. I think what is being proposed is to deprecate the language enum (and turn them from required to optional) and all parameters that use it and to replace it with a string parameter which uses the locale, which the mobile libraries would turn into native objects.

Is the locale parameter optional, or required? It appears to be optional based on the text. If optional, we are left with two optional parameters, and it's possible for neither to be set, which is an error.

I don't think this proposal is adequate for approval as is, but I think the concept is a good one.

[BrettyWhite]

I have two issues with the attached proposal that I would like to see expanded upon.

1. There is no mention of if the locale support in iOS or Android are the same or if they are different. As the goal is to fallback onto that, it needs to be explicitly defined in the proposal (not just links to the developer documentation). This is more of an issue for Core I would imagine, as they would need to expect possibly different parameters depending on the library used.

2. The statement: "Making those parameters optional allows the use of unknown languages" is ambiguous. App developers need to know the language selected to change their strings appropriately. Allowing unknown languages seems like it would be a slippery slope.

[joeljfischer]

@BrettyWhite In answer to your questions, from my understanding:

1. The locale support on android and ios implement an RFC spec http://www.rfc-editor.org/rfc/bcp/bcp47.txt So it would just be sending the language tag strings and those would be turned into native objects that use that spec.

2. The “unknown languages” is a reference to languages outside of the enum, I think, and that’s why using the strings would be an improvement.

[kshala-ford]

@BrettyWhite

There is no mention of if the locale support in iOS or Android are the same or if they are different. As the goal is to fallback onto that, it needs to be explicitly defined in the proposal (not just links to the developer documentation). This is more of an issue for Core I would imagine, as they would need to expect possibly different parameters depending on the library used.

I actually wanted to mention the analogy between Android and iOS locale support with the following sentence.

Instead the locale structure provided by the native phone SDKs should be used that follows locale names defined by the unicode CLDR

I should have explained it more in detail with links to the specs. I'm sorry about that. The BCP-47 mentioned by @joeljfischer is a very good explanation of how the locale identifier is formatted by both platforms. We would just need to follow this format. Linux and other posix conform OS also provide locale support with this format.

The statement: "Making those parameters optional allows the use of unknown languages" is ambiguous. App developers need to know the language selected to change their strings appropriately. Allowing unknown languages seems like it would be a slippery slope.

In my opinion the language support is far too strict compared to the slippery support on the native side. It does not allow adding new languages without causing violation to the spec. There's no difference for the app if .language is set to null or to an unsupported language. It would just stick the current language.

@joeljfischer

Is the locale parameter optional, or required? It appears to be optional based on the text. If optional, we are left with two optional parameters, and it's possible for neither to be set, which is an error.

The reason of deprecating and making parameters optional is to keep the support between different versions of the SDK and core. Once deprecated you have to mark it optional. The time will come where a deprecated parameter will be finally removed. Especially core must be able to work without a deprecated parameter. Otherwise we will never be able to remove them without causing instability to cars on the road. This issue is generic to any mandatory API we want to deprecate.

[joeljfischer]

@kshala-ford I apologize if I was unclear. I'm asking if the new locale parameter is now optional as well, not if the old language parameter is now optional (this is why there should be clear XML changes in the proposal). I think the new locale parameter would have to be required, correct? Which is a major version change? If it's only optional, then it's possible to not send either parameter, and we'll have to handle that case.

[kshala-ford]

@joeljfischer This is for sure a major version change to deprecate language and add locale and it requires another major version change to completely remove language from the API. Personally I believe this will take more than a year (more probably 2+ years).

We have to be careful with mandatory parameters and a mismatch of the API version. Let's say locale will be added to the version 5.0 (hypothetically). If locale becomes mandatory and:

• an app with an 4.x SDK connects to a 5.0 Core
  • the head will receiveRegisterAppInterface.localeDesired not being set although it's mandatory
  • Technically the head unit should reply with a negative response and the app cannot connect
  • The app has no chance to workaround as it doesn't know the Core version before registering
• an app with a 5.0 SDK connects to a 4.x Core
  • the app sent RegisterAppInterface.languageDesired and RegisterAppInterface.localeDesired (which is OK)
  • the app will receive RegisterAppInterfaceResponse.locale not being set although it's mandatory
  • Technically the SDK should find a workaround and set .locale to something meaningful

(this is why there should be clear XML changes in the proposal)

Agree. I would like to defer and update the proposal and add all the changes to the API as XML and add more descriptions of how to deal with omitted parameters on core and mobile side.

[joeljfischer]

@kshala-ford It is not a major version change to deprecate the language enum, to turn required properties to optional, or to add a new property, if it is optional. That's why I was asking about whether locale stuff is mandatory. It sounds like you're saying it's optional, in which case this is a minor version change. I tend to agree with you that this has to be optional, but in that case we are technically under-specifying what needs to be in the RAI, because both language parameters are optional. Perhaps we could add a "locale" enum case to language and if language is set to that it needs to look at the optional locale parameter? Then language could stay required?

[kshala-ford]

@joeljfischer

It is not a major version change to deprecate the language enum, to turn required properties to optional, or to add a new property, if it is optional. That's why I was asking about whether locale stuff is mandatory. It sounds like you're saying it's optional, in which case this is a minor version change.

Yes. locale is optional. The language APIs feel like something fundamental which is why I wrote it would be a major version change.

I tend to agree with you that this has to be optional, but in that case we are technically under-specifying what needs to be in the RAI, because both language parameters are optional.

Do you think language is needed on the RAI request? Personally I would not continue to send the desired language through RAI request. The response is more important but it doesn't make any difference to the app if the language replied is not known of not provided. Both options would produce the same procedure which is ignore the head unit language.

Perhaps we could add a "locale" enum case to language and if language is set to that it needs to look at the optional locale parameter? Then language could stay required?

I disagree. The language parameters should become optional. Otherwise we will not be able to remove it later on.

[joeljfischer]

Yes. locale is optional. The language APIs feel like something fundamental which is why I wrote it would be a major version change.

There is no "feel" with API versioning. See https://semver.org for more detail about how SDL is API versioned.

I tend to agree with you that this has to be optional, but in that case we are technically under-specifying what needs to be in the RAI, because both language parameters are optional.

Do you think language is needed on the RAI request? Personally I would not continue to send the desired language through RAI request. The response is more important but it doesn't make any difference to the app if the language replied is not known of not provided. Both options would produce the same procedure which is ignore the head unit language.

I don't think I can follow what you're saying here, can you clarify? My issue is that we're requiring one of two optional parameters to be set, but that's not specified in the spec because they're optional. Is it okay to not set either? What do we do in that case?

I disagree. The language parameters should become optional. Otherwise, we will not be able to remove it later on.

I doubt we'll ever be able to just remove it. We've generally avoided this kind of hard breaking changes to the RPC spec. I think that for backward compatibility purposes, it should stay required.

[theresalech]

The Steering Committee agreed that this proposal should be revised based on the discussion included in the comments here.

[theresalech]

The proposal has been revised, replacing the Language enum and using the Locale structure/class of the native SDKs. PR: #225

The revised proposal is now in review until August 29, 2017.

[theresalech]

The Steering Committee determined more time was needed to review the impact this proposal has on backwards compatibility. This proposal will remain in review until September 5, 2017.

[kshala-ford]

Based on the following website https://www.science.co.il/language/Locale-codes.php I tried to create a list of how to map locale identifiers to a value of the Language enum.

As per spec (.languageDesired is mandatory) the SDL libraries should fallback to EN_US if the locale identifier cannot be mapped to any other enum value. EDIT: It should not omit this parameter as it would violate the RPC specification.

In mobile development iOS and Android usually separates localizations in "zh-Hans" and "zh-Hant" but not by region. Please take a closer look at the chinese mapping in hope it's an adequate solution.

Locale identifier	Mapped language enum value
Any locale with Arabic ("ar") language	AR_SA
Any locale with Czech ("cs") language	CS_CZ
Any locale with Danish ("da") language	DA_DK
Any locale with German ("de") language	DE_DE
Any locale with Greek ("el") language	EL_GR
"en-AU" locale	EN_AU
"en-GB" locale	EN_GB
"en-IN" locale	EN_IN
"en-SA" locale	EN_SA
Any locale with English ("en") language except "en-AU", "en-GB", "en-IN", "en-SA"	EN_US
Any locale with Spanish ("es") language except "es-MX"	ES_ES
"es-MX" locale	ES_MX
Any locale with Finnish ("fi") language	FI_FI
"fr-CA" locale	FR_CA
Any locale with French ("fr") language except "fr-CA"	FR_FR
Any locale with Hebrew ("he") language	HE_IL
Any locale with Hindi ("hi") language	HI_IN
Any locale with Hungarian ("hu") language	HU_HU
Any locale with Indonesian ("id") language	ID_ID
Any locale with Italian ("it") language	IT_IT
Any locale with Japanese ("ja") language	JA_JP
Any locale with Korean ("ko") language	KO_KR
Any locale with Malay ("ms") language	MS_MY
"nl-BE" locale	NL_BE
Any locale with Dutch ("nl") language except "nl-BE"	NL_NL
Any locale with Norwegian ("no") language	NO_NO
Any locale with Polish ("pl") language	PL_PL
"pt-BR" locale	PT_BR
Any locale with "pt" language except "pt-BR"	PT_PT
Any locale with Romanian ("ro") language	RO_RO
Any locale with Russian ("ru") language	RU_RU
Any locale with Slovak ("sk") language	SK_SK
Any locale with Swedish ("sv") language	SV_SE
Any locale with Thai ("th") language	TH_TH
Any locale with Turkish ("tr") language	TR_TR
Any locale with Ukranian ("uk") language	UK_UA
Any locale with Vietnamese ("vi") language	VI_VN
"zh-TW" (or "zh-Hant" or "zh-Hant-TW")	ZH_TW
Any locale with Chinese ("zh") language (or "zh-Hans") except "zh-TW" or "zh-Hant" or "zh-Hant-TW"	ZH_CN
Any other locale with does not match any of the above options	EN_US

[theresalech]

The Steering Committee has voted to defer this proposal until SDL 0089 has been resolved, as SDL 0061 is plagued by the same problem we're trying to resolve in SDL 0089. After resolving SDL 0089, we should be able to change the language parameter to be of type string instead of language enum (breaking change) to solve a majority of the issues.

[kshala-ford]

To my understanding this proposal is deferred as it's planned to change .language parameters from type Language to type String. I don't recommend to so as it would cause a breaking change and risk of apps not to register (the app may want to register using a locale identifier on an old head unit).

In general I don't recommend to change the type of a parameter as it can be challenging to support both types on the library side.

I agree to still defer this proposal until API versioning is accepted. However we should do so only because we need a procedure how to deprecate the existing language parameters.

[theresalech]

Now that SDL 0089 has been accepted, the Steering Committee has voted to return this proposal for revisions. The author will update the proposal based on the accepted version of SDL 0089, once SDL 0089 has been implemented.

[theresalech]

Author has advised that they are unable to take action on this proposal at this time, and therefore the proposal will be closed. The issue will remain in a returned for revisions status and unlocked so the author can notify the Project Maintainer when it is ready to be revisited.