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

Update lastnpe EEA to 2.4.0 #16875

Merged
merged 23 commits into from
Jul 11, 2024
Merged

Update lastnpe EEA to 2.4.0 #16875

merged 23 commits into from
Jul 11, 2024

Conversation

lsiepel
Copy link
Contributor

@lsiepel lsiepel commented Jun 16, 2024

Resolves: #10396

The reason is that the new version fixes that Optional.orElse can return null values (based on the parameter).
There is quite some code using Optional.orElse expecting the result to be non-null.

In most cases i wrapped the optional in Objects.requireNonNull(..) and in some cases i removed the optional.

At time of writing bindings A-D have been adjusted and i like to get some feedback before proceeding to fix all.
Edit: Moved on to bindings A - K

JFTR: to be merged after 4.2 release as i cannot test all bindings.

@lsiepel lsiepel added the enhancement An enhancement or new feature for an existing add-on label Jun 16, 2024
@lsiepel lsiepel added the work in progress A PR that is not yet ready to be merged label Jun 16, 2024
@J-N-K
Copy link
Member

J-N-K commented Jun 16, 2024

The problem is that we can't do proper null annotations with EEA. Unfortunately there is no will on Eclipse side to fix that. We would need something like "non-null if argument is non-null". This also applies to Map.compute and similar methods.

@lsiepel
Copy link
Contributor Author

lsiepel commented Jun 16, 2024

The problem is that we can't do proper null annotations with EEA. Unfortunately there is no will on Eclipse side to fix that. We would need something like "non-null if argument is non-null". This also applies to Map.compute and similar methods.

Is there some kind of link to that discussion? And more important do you have some thoughts on how to proceed in relation to this PR?

@J-N-K
Copy link
Member

J-N-K commented Jun 16, 2024

Signed-off-by: Leo Siepel <[email protected]>
@morph166955
Copy link
Contributor

I can't validate this any time soon for the AndroidTV/PhillipsTV bit. Is this a Java 17 update or would this still be backwards compatible to Java 11? I'm about to kill off the 3.x compatibility but I haven't yet so I'm trying to keep that in mind still.

@lsiepel
Copy link
Contributor Author

lsiepel commented Jun 16, 2024

I can't validate this any time soon for the AndroidTV/PhillipsTV bit. Is this a Java 17 update or would this still be backwards compatible to Java 11? I'm about to kill off the 3.x compatibility but I haven't yet so I'm trying to keep that in mind still.

As far as i know this should be java 11 compatible. Maybe 4.2 is a good point to move on anyway.

Signed-off-by: Leo Siepel <[email protected]>
@lsiepel lsiepel requested a review from pail23 as a code owner June 23, 2024 14:55
lsiepel added 2 commits June 23, 2024 17:05
Signed-off-by: Leo Siepel <[email protected]>
Signed-off-by: Leo Siepel <[email protected]>
@lsiepel lsiepel requested a review from mrbig as a code owner June 23, 2024 19:20
@lsiepel lsiepel removed the work in progress A PR that is not yet ready to be merged label Jun 23, 2024
Copy link
Contributor

@weymann weymann left a comment

Choose a reason for hiding this comment

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

Fine for sensorcommunity

@wborn wborn added this to the 4.3 milestone Jun 24, 2024
Copy link
Member

@wborn wborn left a comment

Choose a reason for hiding this comment

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

Thanks a lot! 👍

Copy link
Contributor

@bern77 bern77 left a comment

Choose a reason for hiding this comment

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

LGTM for helioseasycontrol

@lsiepel
Copy link
Contributor Author

lsiepel commented Jul 7, 2024

@openhab/add-ons-maintainers now 4.3 development is started, would be nice to have this merged sooner then later.

@wborn
Copy link
Member

wborn commented Jul 8, 2024

Looks like some new code got added that no longer compiles with the newer EEAs.
Can you update that too @lsiepel? After that we can merge the PR.

@lsiepel lsiepel requested a review from jsjames as a code owner July 9, 2024 21:08
@lsiepel
Copy link
Contributor Author

lsiepel commented Jul 10, 2024

@openhab/add-ons-maintainers ping

@wborn wborn merged commit 5e0e9ce into openhab:main Jul 11, 2024
4 of 5 checks passed
@lsiepel lsiepel deleted the update-last-npe branch July 11, 2024 20:58
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Jul 12, 2024
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Jul 18, 2024
pgfeller pushed a commit to pgfeller/openhab-addons that referenced this pull request Sep 29, 2024
Signed-off-by: Leo Siepel <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
joni1993 pushed a commit to joni1993/openhab-addons that referenced this pull request Oct 15, 2024
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade lastnpe EEA to 2.4.0