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

[actions] Add Transformation Actions as a class & Add arg type checking lib #180

Merged

Conversation

florian-h05
Copy link
Contributor

@florian-h05 florian-h05 commented Nov 27, 2022

Fixes #100.

Description

Bundle-size increases from 852 KB to 868 KB, which is really not much.

This adds a Transformation class to explicitely allow access to the
Transformation Actions, which were dynamically exported before.

Signed-off-by: Florian Hotze <[email protected]>
Signed-off-by: Florian Hotze <[email protected]>
Signed-off-by: Florian Hotze <[email protected]>
@florian-h05 florian-h05 added documentation Improvements or additions to documentation enhancement New feature or request labels Nov 27, 2022
@florian-h05 florian-h05 changed the title [WIP] Add Transformation Actions as a class Add Transformation Actions as a class Nov 27, 2022
@florian-h05
Copy link
Contributor Author

@jpg0 Can you review?

@florian-h05 florian-h05 added this to the 2.x.x milestone Nov 27, 2022
@florian-h05 florian-h05 requested a review from jpg0 November 30, 2022 11:15
@florian-h05 florian-h05 changed the title Add Transformation Actions as a class [actions] Add Transformation Actions as a class Dec 2, 2022
@florian-h05 florian-h05 changed the title [actions] Add Transformation Actions as a class [actions] Add Transformation Actions as a class & Add arg type checking lib Dec 2, 2022
@florian-h05
Copy link
Contributor Author

@digitaldan @jpg0 Can one of you please review so we can get this merged before the 3.4.0 feature freeze?

@jpg0
Copy link
Collaborator

jpg0 commented Dec 3, 2022

I would prefer that the Transformation class is pulled out into it's own file, but whatever, that can be done later.

@jpg0
Copy link
Collaborator

jpg0 commented Dec 3, 2022

Does the failing check matter here? I don't know how the docs system works.

@florian-h05
Copy link
Contributor Author

Does the failing check matter here? I don't know how the docs system works.

In this case, no I expect that failure. It is caused, because I reference the JSDoc for the Transformation class from the README. The JSDoc it references is not available online yet, it will be when this PR is merged and JSDoc updates.

@florian-h05
Copy link
Contributor Author

@jpg0 Can we merge then?

Copy link
Contributor

@digitaldan digitaldan left a comment

Choose a reason for hiding this comment

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

LGTM

@florian-h05 florian-h05 merged commit ee6d31a into openhab:main Dec 4, 2022
@florian-h05 florian-h05 deleted the docs-add-transformation-actions branch December 4, 2022 12:56
@jpg0
Copy link
Collaborator

jpg0 commented Dec 4, 2022

FYI the inverse of this work: transformations using ES6 - has not yet been implemented, and still uses Nashorn.

@florian-h05
Copy link
Contributor Author

florian-h05 commented Dec 4, 2022

You refer to the JavaScript Transformation service, right?
I am aware that it is still using Nashorn, the question is if we need to replace it with a new Transformation service addon, or if using the JS Scripting add-on with openhab/openhab-core#2883 is fine.

@jpg0
Copy link
Collaborator

jpg0 commented Dec 4, 2022

Oh wow, there's lots of progress there that I wasn't aware of. Thanks for the pointer.

@florian-h05
Copy link
Contributor Author

@jpg0
There are also docs for this (added today): openhab/openhab-docs#1940.

I’ll add a section to our README to document the usage of JS Scripting for transformations.

florian-h05 added a commit to florian-h05/openhab-js that referenced this pull request Dec 5, 2022
This adds the unit tests for openhab#180.

Signed-off-by: Florian Hotze <[email protected]>
florian-h05 added a commit that referenced this pull request Dec 6, 2022
* [test] Add tests for actions.Transformation
This adds the unit tests for #180.

* [test] Address review for actions.Transformation tests

Signed-off-by: Florian Hotze <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[docs] Transformation action is missing
3 participants