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

Keep channel properties in thing updates #3612

Merged
merged 1 commit into from
Jul 16, 2023
Merged

Conversation

J-N-K
Copy link
Member

@J-N-K J-N-K commented May 13, 2023

Fixes #3610

Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K J-N-K requested a review from a team as a code owner May 13, 2023 18:27
@J-N-K J-N-K added the bug An unexpected problem or unintended behavior of the Core label May 13, 2023
@lolodomo
Copy link
Contributor

Thank you @J-N-K .

I have the feeling you could also fix the problem for label/description with that code ?

        if (label != null) {
            channelBuilder.withLabel(Objects.requireNonNull(label));
        } else {
            channelBuilder.withLabel(oldChannel.getLabel());
        }

        if (description != null) {
            channelBuilder.withDescription(Objects.requireNonNull(description));
        } else {
            channelBuilder.withDescription(oldChannel.getDescription());
        }

@J-N-K
Copy link
Member Author

J-N-K commented May 13, 2023

No. That would most probably be wrong, because the new channel would inherit from the old one instead of the channel-type.

@lolodomo
Copy link
Contributor

No. That would most probably be wrong, because the new channel would inherit from the old one instead of the channel-type.

Oh yes, you're right but that is exactly the same thing for properties, we need to find properties from the new channel, not the old one, that is the use case from @alexf2015, he added channel properties.

@J-N-K
Copy link
Member Author

J-N-K commented May 13, 2023

We could probably add a new section similar to label that adds new properties.

@kaikreuzer
Copy link
Member

@alexf2015 You mentioned here that you don't think that this PR fixes your issue. Shall it be merged nonetheless or do you expect further 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!

@kaikreuzer kaikreuzer merged commit 66dec25 into openhab:main Jul 16, 2023
@kaikreuzer kaikreuzer added this to the 4.0 milestone Jul 16, 2023
@J-N-K J-N-K deleted the properties branch July 16, 2023 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of the Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

thing upgrades: missing support to add/change/remove channel properties
3 participants