-
-
Notifications
You must be signed in to change notification settings - Fork 428
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
[DSL] Expose ColorUtil methods to DSL rules #4410
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM many thanks.
Lets wait for #4401, then we can add |
3a0adf1
to
96ae0df
Compare
Done. |
These methods are just added to the CoreUtil class, correct? |
@holgerfriedrich apropos the thrown exception potentially breaking the API — two things..
openhab-core/bundles/org.openhab.core/src/test/java/org/openhab/core/util/ColorUtilTest.java Line 665 in 963a8d1
|
Refs openhab/openhab-core#4410. Signed-off-by: Florian Hotze <dev@florianhotze.com>
Refs openhab/openhab-core#4410. Signed-off-by: Florian Hotze <dev@florianhotze.com>
@andrewfg thanks for checking, the build seems still fine. I think in tests it is not mandatory to catch all declared exceptions.... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* Expose xyToKelvin and kelvinToXY * Add missing throws declaration Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
96ae0df
to
1e8da74
Compare
Rebased to let CI run on ubuntu-24.04 |
All this looks very good. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
*/ | ||
public static double xyToKelvin(double[] xy) { | ||
public static double xyToKelvin(double[] xy) throws IllegalArgumentException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since an IAE is a runtime exception this should not break code.
Unfortunately, we forgot about the throws declaration in #4367. This is API breaking, you might need to add a catch statement in binding code. We did not find any issues in the add-on repo so far, it still compiles fine.