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 more null annotations to XML processing classes #2775

Merged
merged 1 commit into from
Mar 10, 2022
Merged

Add more null annotations to XML processing classes #2775

merged 1 commit into from
Mar 10, 2022

Conversation

wborn
Copy link
Member

@wborn wborn commented Feb 17, 2022

This adds null annotations to many XML processing classes and a few others.

@wborn wborn requested a review from a team as a code owner February 17, 2022 16:11
@wborn
Add more null annotations to XML processing classes
7bb1800 
This adds null annotations to many XML processing classes and a few others.

Signed-off-by: Wouter Born <github@maindrain.net>
@wborn wborn force-pushed the add-more-null-annotations4 branch from c2722ca to 7bb1800 Compare March 6, 2022 21:12
kaikreuzer
kaikreuzer approved these changes Mar 9, 2022
View reviewed changes
Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

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

Thanks, looks good from what I can see.
One final question, though, as you have added "API breaking" to the PR: I assume this concerns only APIs that are not being used by any (official) add-ons and thus it does not really have an impact. Is this correct?

@wborn
Copy link
Member Author

wborn commented Mar 10, 2022

Yes that's right. The add-ons are not impacted by this. I added the label because I removed the ConverterAssertion utility class. I found that only the assertEndOfType method in that class was used by the ConfigDescriptionConverter . So it seemed like a nice simplification to just move those 3 lines to where they were actually used.

@kaikreuzer
Copy link
Member

Thanks!

@kaikreuzer kaikreuzer merged commit e6ddecc into openhab:main Mar 10, 2022
@kaikreuzer kaikreuzer added this to the 3.3 milestone Mar 10, 2022
@wborn wborn deleted the add-more-null-annotations4 branch March 10, 2022 19:00
@J-N-K J-N-K mentioned this pull request Mar 11, 2022
Closed
J-N-K
J-N-K reviewed Mar 11, 2022
View reviewed changes
...b.core.thing.xml/src/main/java/org/openhab/core/thing/xml/internal/ChannelTypeConverter.java
Comment on lines +77 to +78
return requireNonEmpty((String) nodeIterator.nextValue("item-type", false),
"ChannelType 'itemType' must not be null or empty.");
Copy link
Member

Choose a reason for hiding this comment

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

@wborn I think this change causes #2831. For trigger-channels an empty itemType is ok.

J-N-K
J-N-K reviewed Mar 11, 2022
View reviewed changes
...b.core.thing.xml/src/main/java/org/openhab/core/thing/xml/internal/ChannelTypeConverter.java
Comment on lines +77 to +78
return requireNonEmpty((String) nodeIterator.nextValue("item-type", false),
"ChannelType 'itemType' must not be null or empty.");
Copy link
Member

Choose a reason for hiding this comment

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

@wborn I think this change causes #2831. For trigger-channels an empty itemType is ok.

wborn added a commit to wborn/openhab-core that referenced this pull request Mar 12, 2022
@wborn
Fix trigger channels not found
fed0bf3 
Trigger channel types failed to load because the item type became required with the changes in openhab#2775.
Also adds a unit test for the ThingDescriptionReader to prevent future regressions.

Fixes openhab#2831

Signed-off-by: Wouter Born <github@maindrain.net>
@wborn wborn mentioned this pull request Mar 12, 2022
Merged
kaikreuzer pushed a commit that referenced this pull request Mar 12, 2022
@wborn
Fix trigger channels not found (#2833)
92a6c1c 
Trigger channel types failed to load because the item type became required with the changes in #2775.
Also adds a unit test for the ThingDescriptionReader to prevent future regressions.

Fixes #2831

Signed-off-by: Wouter Born <github@maindrain.net>
splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 12, 2023
@wborn @splatch
Add more null annotations to XML processing classes (openhab#2775)
a67f0ed 
This adds null annotations to many XML processing classes and a few others.

Signed-off-by: Wouter Born <github@maindrain.net>
GitOrigin-RevId: e6ddecc
splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 12, 2023
@wborn @splatch
Fix trigger channels not found (openhab#2833)
4783b32 
Trigger channel types failed to load because the item type became required with the changes in openhab#2775.
Also adds a unit test for the ThingDescriptionReader to prevent future regressions.

Fixes openhab#2831

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

@J-N-K J-N-K J-N-K left review comments

@kaikreuzer kaikreuzer kaikreuzer approved these changes

Assignees
No one assigned
Labels
API breaking
Projects
None yet
Milestone
3.3
Development

Successfully merging this pull request may close these issues.
3 participants
@wborn @kaikreuzer @J-N-K