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

[max] Remove org.apache.commons #14413

Merged
merged 14 commits into from
Jun 2, 2024
Merged

[max] Remove org.apache.commons #14413

merged 14 commits into from
Jun 2, 2024

Conversation

lsiepel
Copy link
Contributor

@lsiepel lsiepel commented Feb 17, 2023

  • Remove dependency on apache.commons

While some unittests have been provided, not everything is tested, so minor refactoring from code perspective.

Test jar 4.1.0 : https://1drv.ms/u/s!AnMcxmvEeupwjq5YgHLYUNfUqbM3ZQ?e=2oAfZ0

Signed-off-by: lsiepel <leosiepel@gmail.com>
@jlaur jlaur changed the title Remove org.apache.commons [max] Remove org.apache.commons Feb 17, 2023
@lsiepel
Copy link
Contributor Author

lsiepel commented Apr 7, 2023

@marcelrv are you able to review?

@jlaur jlaur added the additional testing preferred The change works for the pull request author. A test from someone else is preferred though. label May 9, 2023
@lsiepel
Copy link
Contributor Author

lsiepel commented Jul 29, 2023

@marcelrv Gentle ping

@marcelrv
Copy link
Contributor

@lsiepel sorry I totally missed your earlier inclusion.

I don't see on short notice possibilities to do functional test as I'm redoing my heating and currently don't have the Max operational.

Looking at the code I don't see obvious flaws, so if the tests are passed as well I recommend indeed to merge it

@marcelrv
Copy link
Contributor

LGTM

@lsiepel lsiepel added the work in progress A PR that is not yet ready to be merged label Aug 1, 2023
@lsiepel lsiepel removed work in progress A PR that is not yet ready to be merged additional testing preferred The change works for the pull request author. A test from someone else is preferred though. labels Oct 15, 2023
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
@lsiepel lsiepel requested a review from a team October 20, 2023 17:23
@lsiepel
Copy link
Contributor Author

lsiepel commented Nov 4, 2023

@jlaur can we get this merged before the next milestone?

@jlaur
Copy link
Contributor

jlaur commented Nov 4, 2023

can we get this merged before the next milestone?

I don't think I have reviewed this one, unfortunately I don't have time today. Maybe some other @openhab/add-ons-maintainers can help.

@kaikreuzer
Copy link
Member

@lsiepel As we meanwhile have a StringUtils class in core, should this new method maybe added there?

@lsiepel lsiepel added the awaiting other PR Depends on another PR label Apr 1, 2024
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
@lsiepel lsiepel removed the awaiting other PR Depends on another PR label Apr 7, 2024
@lsiepel lsiepel added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Apr 7, 2024
@lsiepel
Copy link
Contributor Author

lsiepel commented Jun 1, 2024

@jlaur can we get this merged?

Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

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

@jlaur can we get this merged?

Yes, let's get it merged. I finally did a review assisted by ChatGPT to reduce cognitive overload for me when comparing the byte copy implementations. 😄

@jlaur jlaur merged commit c9864d6 into openhab:main Jun 2, 2024
5 checks passed
@jlaur jlaur added this to the 4.2 milestone Jun 2, 2024
@lsiepel lsiepel deleted the max-apache branch June 2, 2024 20:39
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Jun 15, 2024
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Paul Smedley <paul@smedley.id.au>
@jlaur
Copy link
Contributor

jlaur commented Jun 24, 2024

@lsiepel - this is not my area of expertise, but I'm wondering if this line should be removed as well?

org.apache.commons.lang3;version='[3.14.0,3.14.1)',\

@lsiepel
Copy link
Contributor Author

lsiepel commented Jun 24, 2024

@lsiepel - this is not my area of expertise, but I'm wondering if this line should be removed as well?

org.apache.commons.lang3;version='[3.14.0,3.14.1)',\

Yeah, was looking into that as you mentioned in the other post. I think this can safely be removed. I’ll come up with a PR

pgfeller pushed a commit to pgfeller/openhab-addons that referenced this pull request Sep 29, 2024
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Patrik Gfeller <patrik.gfeller@proton.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants