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

[i18n] Refactoring and annotations #1161

Merged
merged 12 commits into from
Oct 27, 2019
Merged

Conversation

cweitkamp
Copy link
Contributor

  • Moved i18n util classes into internal packages
  • Use ConfigI18nLocalizationService instead of ConfigDescriptionI18nUtil in other packages than o.o.c.config.*
  • Refactoring of automation i18n util classes according our default implementation for these util classes (e.g. non static)
  • Added nullness annotations for i18n util classes

Signed-off-by: Christoph Weitkamp github@christophweitkamp.de

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
…Util'

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
…ot successful

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
@wborn wborn added rebuild Triggers the Jenkins PR build and removed rebuild Triggers the Jenkins PR build labels Oct 24, 2019
Copy link
Member

@wborn wborn left a comment

Choose a reason for hiding this comment

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

Thanks! Setting the TranslationProvider using the constructor in the utility classes helps a lot with reducing the number of method parameters and simplifying the code.

Did you also consider creating components out of those utility classes (ModuleI18nUtil, ModuleTypeI18nUtil, RuleTemplateI18nUtil)? It might make the classes using those Util classes easier to unit test when instead of the Util classes some Mocks can be injected.

I also have one small comment below.

@wborn wborn added the i18n/l10n Internationalization/Localization label Oct 25, 2019
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
@cweitkamp
Copy link
Contributor Author

Did you also consider creating components out of those utility classes

No, I did not. We have different i18n services / components which should be used by other bundles if they need localized entities. My understandig of those utility classes is that they are internal and only be used by the i18n services itself as singleton object - more or less to reduce lines of code / to source out the code for better maintainability of the i18n services.

Copy link
Member

@wborn wborn left a comment

Choose a reason for hiding this comment

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

LGTM now!

@wborn wborn merged commit 12d91f7 into openhab:master Oct 27, 2019
@cweitkamp cweitkamp deleted the feature-i18n-utils branch October 27, 2019 10:18
@wborn wborn added this to the 2.5 milestone Oct 27, 2019
Rosi2143 pushed a commit to Rosi2143/openhab-core that referenced this pull request Dec 26, 2020
Fixes openhab#1161

Signed-off-by: Wouter Born <github@maindrain.net>
splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 11, 2023
* Added nullness annotations to 'RuleTemplateI18nUtil'
* Added nullness annotations to 'ConfigDescriptionGroupI18nUtil'
* Moved 'ConfigDescriptionI18nUtil' into internal package
* Moved 'ConfigDescriptionGroupI18nUtil' into internal package
* Moved 'ThingTypeI18nUtil' into internal package
* Use 'ConfigI18nLocalizationService' instead of 'ConfigDescriptionI18nUtil'
* Use default labels / descriptions if application of localization is not successful
* Resolved itest.bndrun files

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
GitOrigin-RevId: 12d91f7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i18n/l10n Internationalization/Localization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants