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

Remove deprecated method of 'ConfigOptionProvider' #1541

Merged
merged 2 commits into from
Jul 11, 2020
Merged

Remove deprecated method of 'ConfigOptionProvider' #1541

merged 2 commits into from
Jul 11, 2020

Conversation

cweitkamp
Copy link
Contributor

@cweitkamp cweitkamp commented Jul 9, 2020 •
edited
Loading

  • Remove deprecated method of ConfigOptionProvider

Related to #1408

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

Depends on openhab/openhab-addons#8093

@cweitkamp cweitkamp added work in progress A PR that is not yet ready to be merged API breaking labels Jul 9, 2020
@wborn wborn mentioned this pull request Jul 9, 2020
Closed
20 tasks
cweitkamp
cweitkamp commented Jul 9, 2020
View reviewed changes
...penhab.core.config.core/src/main/java/org/openhab/core/config/core/ConfigOptionProvider.java Outdated
Collection<ParameterOption> getParameterOptions(URI uri, String param, @Nullable Locale locale);
// @Deprecated
// @Nullable
// default Collection<ParameterOption> getParameterOptions(URI uri, String param, @Nullable Locale locale) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This method needs a little attention. If we remove it now it will break the openHAB 3 Add-ons build.

Copy link
Member

Choose a reason for hiding this comment

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

So if I get it right, your plan now is to keep it as a deprecated "default" method in place and remove it later?
Which add-ons will break?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that was the initial idea. We are talking about three bindings:

  • Amazon Dash Button
  • GPSTracker
  • SmartMeter

There are two other options:

  1. We could exclude them from openHAB 3 Add-ons build.

or

  1. We could move them to "master"-branch and migrate them right now.

Copy link
Member

Choose a reason for hiding this comment

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

We are talking about three bindings

Shouldn't it be fairly easy to adapt them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shouldn't it be fairly easy to adapt them?

Yes, it is. But we cannot use the same code to compile against OHC 2.5 and OHC 3 (after this PR got merged). If we remove the deprecated method in the bindings it will fail on OH 2.5.x Add-ons build and if we remove the deprecated method here it will fail on OH 3 Add-ons build.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I see. I had missed the fact that it is a "provider" interface and not a "consumer" interface.
So yeah, I think your approach is just fine - let's leave it in as a default method and remove it once we move all add-ons to master. Do you keep track of those open changes somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We keep track of it in #1408. I added a note.

@cweitkamp cweitkamp added the awaiting other PR Depends on another PR label Jul 9, 2020
@cweitkamp cweitkamp mentioned this pull request Jul 9, 2020
Merged
@cweitkamp cweitkamp changed the title [WIP] Remove deprecated method in 'ConfigOptionProvider' [WIP] Remove deprecated method of 'ConfigOptionProvider' Jul 9, 2020
@cweitkamp cweitkamp removed the awaiting other PR Depends on another PR label Jul 10, 2020
@cweitkamp
Remove deprecated method in 'ConfigOptionProvider'
ccba621 
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
@cweitkamp
Remove deprecated method in 'ConfigOptionProvider'
9b96b6c 
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
@cweitkamp cweitkamp force-pushed the feature-remove-deprecated-configoptionprovider-method branch from a7575ce to 9b96b6c Compare July 10, 2020 10:05
@cweitkamp cweitkamp removed the work in progress A PR that is not yet ready to be merged label Jul 10, 2020
@cweitkamp cweitkamp changed the title [WIP] Remove deprecated method of 'ConfigOptionProvider' Remove deprecated method of 'ConfigOptionProvider' Jul 10, 2020
@kaikreuzer kaikreuzer merged commit 991ccd6 into openhab:master Jul 11, 2020
@cweitkamp cweitkamp deleted the feature-remove-deprecated-configoptionprovider-method branch July 12, 2020 06:47
@cweitkamp cweitkamp added this to the 3.0 milestone Jul 12, 2020
wborn added a commit to wborn/openhab-core that referenced this pull request Sep 23, 2020
@wborn
Remove deprecated default methods
4a396a1 
Removes:

* ConfigOptionProvider.getParameterOptions(URI, String, Locale) (see also openhab#1541)
* DiscoveryListener.removeOlderResults(DiscoveryService, long, Collection<ThingTypeUID>)

Related to openhab#1408

Signed-off-by: Wouter Born <github@maindrain.net>
wborn added a commit to wborn/openhab-core that referenced this pull request Sep 23, 2020
@wborn
Remove deprecated default methods
861e3e7 
Removes:

* ConfigOptionProvider.getParameterOptions(URI, String, Locale) (see also openhab#1541)
* DiscoveryListener.removeOlderResults(DiscoveryService, long, Collection<ThingTypeUID>)

Related to openhab#1408

Signed-off-by: Wouter Born <github@maindrain.net>
@wborn wborn mentioned this pull request Sep 23, 2020
Merged
kaikreuzer pushed a commit that referenced this pull request Sep 23, 2020
@wborn
Remove deprecated default methods (#1667)
d33598a 
Removes:

* ConfigOptionProvider.getParameterOptions(URI, String, Locale) (see also #1541)
* DiscoveryListener.removeOlderResults(DiscoveryService, long, Collection<ThingTypeUID>)

Related to #1408

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
@cweitkamp @splatch
Remove deprecated method of 'ConfigOptionProvider' (openhab#1541)
8510d21 
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
GitOrigin-RevId: 991ccd6
splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 11, 2023
@wborn @splatch
Remove deprecated default methods (openhab#1667)
86b8344 
Removes:

* ConfigOptionProvider.getParameterOptions(URI, String, Locale) (see also openhab#1541)
* DiscoveryListener.removeOlderResults(DiscoveryService, long, Collection<ThingTypeUID>)

Related to openhab#1408

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

@kaikreuzer kaikreuzer kaikreuzer approved these changes

Assignees
No one assigned
Labels
API breaking
Projects
None yet
Milestone
3.0.0.M1
Development

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