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

[upnpcontrol] Remove org.apache.common #14439

Merged
merged 1 commit into from
Feb 19, 2023
Merged

[upnpcontrol] Remove org.apache.common #14439

merged 1 commit into from
Feb 19, 2023

Conversation

lsiepel
Copy link
Contributor

@lsiepel lsiepel commented Feb 18, 2023

  • Remove dependency on org.apache.commons

Untested, minor refactoring only from code perspective

@lsiepel
Remove org.apache.common
a0b5b64 
Signed-off-by: lsiepel <leosiepel@gmail.com>
@lsiepel lsiepel mentioned this pull request Feb 18, 2023
Open
@mherwege
Copy link
Contributor

Shouldn’t this be turned into a XmlUtils class in core now? See the original PR of this binding, and also a comment in the issue here. The Sonos binding has the same requirement.

@mherwege
Copy link
Contributor

And maybe have a look at this. I do think string escaping/unescaping is a general requirement when working with integrations, and a more general solution than doing it separately for every binding could make sense.

@lsiepel
Copy link
Contributor Author

lsiepel commented Feb 19, 2023

Agree, but after more than two and a half years these utility classes have not been added yet. So I decided to add these PR’s to atleast get this going. I hope it get’s merged and the discussion gets boosted at the topic you mentioned.
From here it should not be hard to consolidate these utility classes to core. Hope you can confirm the implementation looks fit.

@mherwege
Copy link
Contributor

Yes, LGTM

lolodomo
lolodomo approved these changes Feb 19, 2023
View reviewed changes
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

LGTM, thank you

@lolodomo lolodomo merged commit 833578b into openhab:main Feb 19, 2023
@lolodomo lolodomo added the enhancement An enhancement or new feature for an existing add-on label Feb 19, 2023
@lolodomo lolodomo added this to the 4.0 milestone Feb 19, 2023
@lsiepel lsiepel deleted the upnpcontrol-apache branch February 19, 2023 20:07
@lsiepel lsiepel mentioned this pull request Feb 19, 2023
Merged
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Feb 28, 2023
@lsiepel @nemerdaud
[upnpcontrol] Remove org.apache.common (openhab#14439)
fb28df0 
Signed-off-by: lsiepel <leosiepel@gmail.com>
@jlaur jlaur removed the enhancement An enhancement or new feature for an existing add-on label Mar 10, 2023
renescherer pushed a commit to renescherer/openhab-addons that referenced this pull request Mar 23, 2023
@lsiepel @renescherer
[upnpcontrol] Remove org.apache.common (openhab#14439)
e3faebb 
Signed-off-by: lsiepel <leosiepel@gmail.com>
FordPrfkt pushed a commit to FordPrfkt/openhab-addons that referenced this pull request Apr 20, 2023
@lsiepel @FordPrfkt
[upnpcontrol] Remove org.apache.common (openhab#14439)
7af3629 
Signed-off-by: lsiepel <leosiepel@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lolodomo lolodomo lolodomo approved these changes

@mherwege mherwege Awaiting requested review from mherwege mherwege is a code owner

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

Successfully merging this pull request may close these issues.
4 participants
@lsiepel @mherwege @lolodomo @jlaur