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.

Sign up for GitHub

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

Jump to bottom

Add null annotations to registries #1134

Merged
merged 1 commit into from
Oct 23, 2019
Merged

Add null annotations to registries #1134

merged 1 commit into from
Oct 23, 2019

Conversation

wborn
Copy link
Member

@wborn wborn commented Oct 16, 2019 •
edited
Loading

Adds null annotations to all registries, the interfaces they implement and a few other classes.

There's also a PR to update the add-ons for this: https://github.com/openhab/openhab2-addons/pull/6233

@wborn wborn mentioned this pull request Oct 16, 2019
Merged
@wborn wborn force-pushed the registries-null-annotations branch 2 times, most recently from 0ea8d24 to d0f8846 Compare October 18, 2019 21:20
cweitkamp
cweitkamp reviewed Oct 19, 2019
View reviewed changes
Copy link
Contributor

@cweitkamp cweitkamp left a comment

Choose a reason for hiding this comment

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

I could not yet do a full review but stumbled across some parts of this too while annotating the org.openhab.core.config.core bundle which I would like to discuss with you (see my latest PR #1152).

...omation/src/main/java/org/openhab/core/automation/internal/provider/i18n/ModuleI18nUtil.java Outdated Show resolved Hide resolved
...on/src/main/java/org/openhab/core/automation/module/provider/i18n/ModuleTypeI18nService.java Outdated Show resolved Hide resolved
@wborn wborn mentioned this pull request Oct 19, 2019
Closed
@wborn wborn force-pushed the registries-null-annotations branch 2 times, most recently from a2205a8 to e6501c9 Compare October 20, 2019 08:51
cweitkamp
cweitkamp reviewed Oct 22, 2019
View reviewed changes
Copy link
Contributor

@cweitkamp cweitkamp left a comment

Choose a reason for hiding this comment

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

I did a full review and now I understand the challenge with this. Took quite some time. I added some minor remarks and have to ask the same question regarding the bundle like before for the ConfigDescriptionI18nUtil.

.../main/java/org/openhab/core/automation/internal/provider/AbstractResourceBundleProvider.java Outdated Show resolved Hide resolved
...tomation/src/main/java/org/openhab/core/automation/internal/type/ModuleTypeRegistryImpl.java Outdated Show resolved Hide resolved
...tomation/src/main/java/org/openhab/core/automation/internal/type/ModuleTypeRegistryImpl.java Outdated Show resolved Hide resolved
...hab.core.automation/src/main/java/org/openhab/core/automation/template/TemplateRegistry.java Outdated Show resolved Hide resolved
...penhab.core.ui/src/main/java/org/eclipse/smarthome/ui/internal/items/ItemUIRegistryImpl.java Outdated Show resolved Hide resolved
...fig.core/src/main/java/org/eclipse/smarthome/config/core/i18n/ConfigDescriptionI18nUtil.java Outdated Show resolved Hide resolved
@wborn wborn force-pushed the registries-null-annotations branch from aa935b4 to 132a671 Compare October 22, 2019 19:27
@wborn
Add null annotations to registries
f833408 
Adds null annotations to all registries, the interfaces they implement and a few other classes.

Also-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Wouter Born <github@maindrain.net>
@wborn wborn force-pushed the registries-null-annotations branch from 132a671 to f833408 Compare October 22, 2019 20:04
cweitkamp
cweitkamp approved these changes Oct 22, 2019
View reviewed changes
Copy link
Contributor

@cweitkamp cweitkamp left a comment

Choose a reason for hiding this comment

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

LGTM. Great job. Thanks.

@wborn wborn added rebuild Triggers the Jenkins PR build and removed rebuild Triggers the Jenkins PR build labels Oct 22, 2019
@cweitkamp cweitkamp merged commit 2430256 into openhab:master Oct 23, 2019
@cweitkamp cweitkamp added this to the 2.5 milestone Oct 23, 2019
cweitkamp
cweitkamp reviewed Oct 23, 2019
View reviewed changes
.../main/java/org/openhab/core/automation/internal/provider/AbstractResourceBundleProvider.java
@@ -406,6 +399,7 @@ protected Bundle getBundle(String uid) {
uri = new URI(prefix + ":" + uid + ".name");
} catch (URISyntaxException e) {
logger.error("Constructed invalid uri '{}:{}.name'", prefix, uid, e);
return Collections.emptyList();
Copy link
Contributor

Choose a reason for hiding this comment

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

We should return the default values here. I fixed it in https://github.com/openhab/openhab-core/pull/1161/files#diff-d90b601b2b68bbe8017ba644c98c6da1R387.

@wborn wborn deleted the registries-null-annotations branch December 8, 2019 23:19
@J-N-K J-N-K mentioned this pull request Nov 15, 2022
Merged
splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 11, 2023
@wborn @splatch
Add null annotations to registries (openhab#1134)
c27e88b 
Adds null annotations to all registries, the interfaces they implement and a few other classes.

Also-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Wouter Born <github@maindrain.net>
GitOrigin-RevId: 2430256
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cweitkamp cweitkamp cweitkamp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.5
Development

Successfully merging this pull request may close these issues.
2 participants
@wborn @cweitkamp