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

[homematic] Replaced deprecated constructors #7352

Merged
merged 2 commits into from
Apr 13, 2020
Merged

[homematic] Replaced deprecated constructors #7352

merged 2 commits into from
Apr 13, 2020

Conversation

MHerbst
Copy link
Contributor

@MHerbst MHerbst commented Apr 13, 2020

The binding used some deprecated constructors. These usages have been replaced to allow the removal of these constructors. See also: openhab/openhab-core#1414

@MHerbst MHerbst requested a review from gerrieg as a code owner April 13, 2020 09:29
@MHerbst
Replaced deprecated constructors
445c519 
Signed-off-by: Martin Herbst <develop@mherbst.de>
@TravisBuddy
Copy link

Travis tests were successful

Hey @MHerbst,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

cweitkamp
cweitkamp reviewed Apr 13, 2020
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.

Thank you for the quick work. On request - see comment:

...ic/src/main/java/org/openhab/binding/homematic/internal/type/HomematicTypeGeneratorImpl.java Outdated
@@ -301,11 +303,10 @@ public EventOption createOption(String value, String description) {
return new EventOption(value, description);
}
}));

}
channelType = new ChannelType(channelTypeUID, !MetadataUtils.isStandard(dp), itemType, channelKind, label,
Copy link
Contributor

Choose a reason for hiding this comment

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

While you are already touching the code, may I ask you to replace this constructor by ChannelTypeBuilder too?

Copy link
Contributor Author

@MHerbst MHerbst Apr 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I have expected this comment :-). Unfortunately I could not simply replace it by the ChannelTypeBuilder because the parameter channelKind is an enum here. The constructor call is used to create State channels as well as Trigger channels. This means some additional changes are necessary and I preferred to do this in a separate step to get rid of these deprecated constructors as soon as possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

I get your point and of course it is an improvement but the thing is that exactly this ctor will be removed in openhab/openhab-core#1414 and thus this PR will not fix the build of the OH3 add-on port.

Isn't this code snippet a simple solution:

                        }));
                channelType = ChannelTypeBuilder.trigger(channelTypeUID, label).with....build();
            }
            channelType = ChannelTypeBuilder.state(channelTypeUID, label, itemType).with....build();
        }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, this constructor is currently not marked as deprecated. Therefore I thought I could use it.
I have changed it.

@openhab openhab deleted a comment from TravisBuddy Apr 13, 2020
@MHerbst
Use of builder instead of constructor
ffad50f 
Signed-off-by: Martin Herbst <develop@mherbst.de>
@TravisBuddy
Copy link

Travis tests were successful

Hey @MHerbst,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

cweitkamp
cweitkamp approved these changes Apr 13, 2020
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. Thanks.

Working nicely now.

@cweitkamp cweitkamp changed the title Replaced deprecated constructors [homematic] Replaced deprecated constructors Apr 13, 2020
@cweitkamp cweitkamp added this to the 2.5.4 milestone Apr 13, 2020
@cweitkamp cweitkamp merged commit 83eabef into openhab:2.5.x Apr 13, 2020
yfre pushed a commit to yfre/openhab-addons that referenced this pull request Apr 27, 2020
@MHerbst
[homematic] Replaced deprecated constructors and use builders (openha…
2bc67d8 
…b#7352)

* Replaced deprecated constructors
* Use of builder instead of constructor

Signed-off-by: Martin Herbst <develop@mherbst.de>
Signed-off-by: Eugen Freiter <freiter@gmx.de>
markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request May 29, 2020
@MHerbst @markus7017
[homematic] Replaced deprecated constructors and use builders (openha…
f17274b 
…b#7352)

* Replaced deprecated constructors
* Use of builder instead of constructor

Signed-off-by: Martin Herbst <develop@mherbst.de>
LoungeFlyZ pushed a commit to LoungeFlyZ/openhab2-addons that referenced this pull request Jun 8, 2020
@MHerbst @LoungeFlyZ
[homematic] Replaced deprecated constructors and use builders (openha…
e5402a5 
…b#7352)

* Replaced deprecated constructors
* Use of builder instead of constructor

Signed-off-by: Martin Herbst <develop@mherbst.de>
@MHerbst MHerbst deleted the improvements branch June 13, 2020 09:10
J-N-K pushed a commit to J-N-K/openhab-addons that referenced this pull request Jul 14, 2020
@MHerbst @J-N-K
[homematic] Replaced deprecated constructors and use builders (openha…
9696f2f 
…b#7352)

* Replaced deprecated constructors
* Use of builder instead of constructor

Signed-off-by: Martin Herbst <develop@mherbst.de>
CSchlipp pushed a commit to CSchlipp/openhab-addons that referenced this pull request Jul 26, 2020
@MHerbst @CSchlipp
[homematic] Replaced deprecated constructors and use builders (openha…
b066d0a 
…b#7352)

* Replaced deprecated constructors
* Use of builder instead of constructor

Signed-off-by: Martin Herbst <develop@mherbst.de>
Signed-off-by: CSchlipp <christian@schlipp.de>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
@MHerbst @andrewfg
[homematic] Replaced deprecated constructors and use builders (openha…
7d4827b 
…b#7352)

* Replaced deprecated constructors
* Use of builder instead of constructor

Signed-off-by: Martin Herbst <develop@mherbst.de>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
@MHerbst @andrewfg
[homematic] Replaced deprecated constructors and use builders (openha…
724cbea 
…b#7352)

* Replaced deprecated constructors
* Use of builder instead of constructor

Signed-off-by: Martin Herbst <develop@mherbst.de>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
@MHerbst @andrewfg
[homematic] Replaced deprecated constructors and use builders (openha…
864205f 
…b#7352)

* Replaced deprecated constructors
* Use of builder instead of constructor

Signed-off-by: Martin Herbst <develop@mherbst.de>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
@MHerbst @andrewfg
[homematic] Replaced deprecated constructors and use builders (openha…
7bb6d8f 
…b#7352)

* Replaced deprecated constructors
* Use of builder instead of constructor

Signed-off-by: Martin Herbst <develop@mherbst.de>
DaanMeijer pushed a commit to DaanMeijer/openhab-addons that referenced this pull request Sep 1, 2020
@MHerbst @DaanMeijer
[homematic] Replaced deprecated constructors and use builders (openha…
5f31767 
…b#7352)

* Replaced deprecated constructors
* Use of builder instead of constructor

Signed-off-by: Martin Herbst <develop@mherbst.de>
Signed-off-by: Daan Meijer <daan@studioseptember.nl>
@cweitkamp cweitkamp mentioned this pull request Sep 12, 2020
Merged
markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request Sep 19, 2020
@MHerbst @markus7017
[homematic] Replaced deprecated constructors and use builders (openha…
768b926 
…b#7352)

* Replaced deprecated constructors
* Use of builder instead of constructor

Signed-off-by: Martin Herbst <develop@mherbst.de>
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

@gerrieg gerrieg Awaiting requested review from gerrieg

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

Successfully merging this pull request may close these issues.
3 participants
@MHerbst @TravisBuddy @cweitkamp