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

Add support for installing openhab & openhab_rules_tools from npm #1714

Merged
merged 20 commits into from Sep 21, 2022

Conversation

florian-h05
Copy link
Contributor

Fixes #1651.

This PR adds a menu option to install the openHAB JavaScript library and openhab_rules_tools from @rkoshak.

@florian-h05 florian-h05 force-pushed the add-openhab-node-packages branch 5 times, most recently from a85c42e to 6430bcc Compare September 13, 2022 13:09
@florian-h05
Copy link
Contributor Author

@mstormi
This is nearly ready for merging.

At every start of the openhabian-config tool, it checks the following for both openhab & openhab_rules_tools:

  1. Does the package folder exist? If not, exit.
  2. Run npm outdated and check for the package name in the output. If the package is mentioned in the output, an update is available.
  3. Ask the user for updating.

In the openhabian-config tool, you can install both packages under 40 | openHAB Related.

I only need your help where to put the command for auto-installing openhab_rules_tools.

functions/menu.bash Outdated Show resolved Hide resolved
openhabian-setup.sh Show resolved Hide resolved
Copy link

@rkoshak rkoshak left a comment

Choose a reason for hiding this comment

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

Just a few quick comments

functions/menu.bash Outdated Show resolved Hide resolved
functions/menu.bash Outdated Show resolved Hide resolved
functions/nodejs-apps.bash Outdated Show resolved Hide resolved
@florian-h05 florian-h05 requested review from rkoshak and mstormi and removed request for mstormi and rkoshak September 14, 2022 14:56
@florian-h05
Copy link
Contributor Author

@rkoshak @mstormi
Sorry for the chaos with the review requests.
I‘m somehow not able to request a re-review from both of you using the „request review“ button.

@rkoshak
Copy link

rkoshak commented Sep 14, 2022

Short of being able to test this myself all I can say is LGTM.

@florian-h05
Copy link
Contributor Author

I tested all recent changes just a few minutes ago, everything works.

Should be ready for merging @mstormi.

@mstormi
Copy link
Contributor

mstormi commented Sep 17, 2022

please rebase to include a fix I've just to main (it should make the installation tests pass)

functions/menu.bash Outdated Show resolved Hide resolved
functions/menu.bash Outdated Show resolved Hide resolved
functions/nodejs-apps.bash Show resolved Hide resolved
functions/nodejs-apps.bash Outdated Show resolved Hide resolved
functions/nodejs-apps.bash Outdated Show resolved Hide resolved
openhabian-setup.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@mstormi mstormi left a comment

Choose a reason for hiding this comment

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

please also rebase

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Fixes openhab#1651.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
This change indicates that openhab-js is included in JS Scripting and
can be upgraded manually, but does not have to be installed manually.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Fixes an issue, where the regex matches on a different package, that included the package name

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05
Copy link
Contributor Author

@mstormi
I addressed your review and rebased, retested all current changes and everything works.
From my side this is ready for merging, feel free to re-review.

@florian-h05 florian-h05 requested review from mstormi and removed request for rkoshak September 21, 2022 15:22
Copy link
Contributor

@mstormi mstormi left a comment

Choose a reason for hiding this comment

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

almost there

functions/menu.bash Outdated Show resolved Hide resolved
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05
Copy link
Contributor Author

@mstormi
I updated the whiptail menu size:

IMG_1715

Copy link
Contributor

@mstormi mstormi left a comment

Choose a reason for hiding this comment

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

LGTM thanks

@mstormi mstormi merged commit 96c045b into openhab:main Sep 21, 2022
@florian-h05 florian-h05 deleted the add-openhab-node-packages branch September 21, 2022 21:41
florian-h05 added a commit to openhab/openhab-js that referenced this pull request Sep 25, 2022
This updates the README to explain the openhab-js installation using
the openhabian-config tool.
Reference openhab/openhabian#1714.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@mstormi
Copy link
Contributor

mstormi commented Oct 11, 2022 via email

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.

Add openhab_rules_tools
3 participants