Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] Added i18n feature for profiles #785

Merged
merged 3 commits into from
May 9, 2019

Conversation

cweitkamp
Copy link
Contributor

  • Added i18n feature for profiles

One comment: There is an internal class LocalizedProfileTypeKey which is used in a very similar way for other translatable objects (e.g. https://github.com/openhab/openhab-core/blob/master/bundles/org.openhab.core.thing/src/main/java/org/eclipse/smarthome/core/thing/DefaultSystemChannelTypeProvider.java#L281-L328 and https://github.com/openhab/openhab-core/blob/master/bundles/org.openhab.core.config.xml/src/main/java/org/eclipse/smarthome/config/xml/AbstractXmlBasedProvider.java#L40-L79). I am not very happy with that and think about a refactoring. Let me know your opinion.

Signed-off-by: Christoph Weitkamp [email protected]

@cweitkamp cweitkamp added the i18n/l10n Internationalization/Localization label May 2, 2019
Signed-off-by: Christoph Weitkamp <[email protected]>
@cweitkamp cweitkamp force-pushed the feature-profile-i18n branch 2 times, most recently from 00c0925 to 270b6f5 Compare May 4, 2019 08:52
Signed-off-by: Christoph Weitkamp <[email protected]>
@cweitkamp cweitkamp force-pushed the feature-profile-i18n branch 3 times, most recently from d0af285 to f144438 Compare May 6, 2019 15:32
Signed-off-by: Christoph Weitkamp <[email protected]>
@cweitkamp cweitkamp force-pushed the feature-profile-i18n branch from f144438 to d99224d Compare May 6, 2019 15:34
@maggu2810
Copy link
Contributor

maggu2810 commented May 7, 2019

What about a LocalizedKey<K,V>?

  • SystemProfileFactory.java.LocalizedProfileTypeKey can be replaced by LocalizedKey<UID, @Nullable String>
  • DefaultSystemChannelTypeProvider.LocalizedChannelTypeKey can be replaced by LocalizedKey<UID, @Nullable String>
  • AbstractXmlBasedProvider.LocalizedKey can be replaced by LocalizedKey<Object, @Nullable String>

Okay, while reading the classes, I assume we don't need generics at all.

LocalizedKey can use two members:

  • final Object key
  • final @Nullable String locale

The type of the key does not matter.

@cweitkamp
Copy link
Contributor Author

Yes, sounds good. Moving it to an own class in the i18n package.

Another question which came to my mind was if it is the right place to use the localized caches in the XXXTypeProviders. Or should we maybe move them over to the XXXTypeI18nLocalizationServices components which makes imho much more sense because the services could be used in multiple places and the benefits may be even bigger.

Independently of the final approach we should do it in a separate PR to keep the history clean.

@cweitkamp cweitkamp added rebuild Triggers the Jenkins PR build and removed rebuild Triggers the Jenkins PR build labels May 8, 2019
@maggu2810
Copy link
Contributor

Independently of the final approach we should do it in a separate PR to keep the history clean.

👍

IMHO also the LocalizedKey can be done in another PR as it is not about i18n feature in profiles itself.
Will you volunteer for that improvement? 😉

@maggu2810 maggu2810 merged commit b5f33d3 into openhab:master May 9, 2019
@cweitkamp cweitkamp deleted the feature-profile-i18n branch May 9, 2019 05:51
@cweitkamp
Copy link
Contributor Author

Will you volunteer for that improvement?

Of course. I already have it in the pipe.

@wborn wborn added this to the 2.5 milestone Jul 30, 2019
@cweitkamp cweitkamp added the enhancement An enhancement or new feature of the Core label Dec 3, 2019
@cweitkamp cweitkamp changed the title Added i18n feature for profiles [core] Added i18n feature for profiles Dec 3, 2019
splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 11, 2023
Signed-off-by: Christoph Weitkamp <[email protected]>
GitOrigin-RevId: b5f33d3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature of the Core i18n/l10n Internationalization/Localization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants