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.

Sign up for GitHub

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

Jump to bottom

Update install-gui.sh to use check Node 18 and npm 9 #16424

Merged
merged 6 commits into from
Sep 26, 2023
Merged

Update install-gui.sh to use check Node 18 and npm 9 #16424

merged 6 commits into from
Sep 26, 2023

Conversation

paninaro
Copy link
Contributor

Purpose:

The GUI has been updated to use Node 18 and npm 9. This change updates the install-gui.sh to verify that Node 18 and npm 9 are installed.

@paninaro paninaro added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label Sep 26, 2023
@paninaro paninaro requested a review from a team as a code owner September 26, 2023 00:44
hoffmang9
hoffmang9 previously approved these changes Sep 26, 2023
View reviewed changes
Copy link
Member

@hoffmang9 hoffmang9 left a comment

Choose a reason for hiding this comment

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

Lgtm

wallentx
wallentx reviewed Sep 26, 2023
View reviewed changes
install-gui.sh Outdated Show resolved Hide resolved
arvidn
arvidn previously approved these changes Sep 26, 2023
View reviewed changes
@ChiaMineJP ChiaMineJP dismissed stale reviews from arvidn and hoffmang9 via 332e530 September 26, 2023 17:18
hoffmang9
hoffmang9 previously approved these changes Sep 26, 2023
View reviewed changes
Copy link
Member

@hoffmang9 hoffmang9 left a comment

Choose a reason for hiding this comment

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

lgtm

wallentx
wallentx previously approved these changes Sep 26, 2023
View reviewed changes
@ChiaMineJP
Copy link
Contributor

@hoffmang9 @wallentx
Sorry everyone, I need to push one more commit to suppress this warning:
image

@ChiaMineJP ChiaMineJP dismissed stale reviews from wallentx and hoffmang9 via b6cac37 September 26, 2023 18:00
@ChiaMineJP
Copy link
Contributor

Also needed to update this lines of code:
image

@ChiaMineJP
Copy link
Contributor

ChiaMineJP commented Sep 26, 2023 •
edited
Loading

Tested both install-gui.sh and start-gui.sh on Ubuntu 22.04 and confirmed it works.
(Updated)
Tested on MacOS Ventura 13.5.1 and it works

hoffmang9
hoffmang9 previously approved these changes Sep 26, 2023
View reviewed changes
Copy link
Member

@hoffmang9 hoffmang9 left a comment

Choose a reason for hiding this comment

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

lgtm

@ChiaMineJP
Copy link
Contributor

ChiaMineJP commented Sep 26, 2023 •
edited
Loading

Test on very old NodeJS/npm failed with the newer format of package-lock.json. (Thank you William)
Execuse me, but one-last commit will be pushed.
William will test on the new commit.
Please wait until William confirmed the new script.

wallentx
wallentx approved these changes Sep 26, 2023
View reviewed changes
Copy link
Contributor

@wallentx wallentx left a comment

Choose a reason for hiding this comment

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

Tested on Arch:

  • With a good version of node/npm installed ✅
  • With an old version of node/npm installed ✅
  • Without node/npm installed ✅

wjblanke
wjblanke approved these changes Sep 26, 2023
View reviewed changes
Copy link
Contributor

@wjblanke wjblanke left a comment

Choose a reason for hiding this comment

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

aok

@wallentx wallentx merged commit 8b4b2ca into release/2.1.0 Sep 26, 2023
544 checks passed
@wallentx wallentx deleted the paninaro.node18_install_gui branch September 26, 2023 22:30
This was referenced Oct 28, 2023
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hoffmang9 hoffmang9 hoffmang9 approved these changes

@wallentx wallentx wallentx approved these changes

@cmmarslender cmmarslender cmmarslender approved these changes

@wjblanke wjblanke wjblanke approved these changes

@emlowe emlowe Awaiting requested review from emlowe

@ChiaMineJP ChiaMineJP Awaiting requested review from ChiaMineJP

@arvidn arvidn Awaiting requested review from arvidn

Assignees
No one assigned
Labels
Changed Required label for PR that categorizes merge commit message as "Changed" for changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@paninaro @ChiaMineJP @arvidn @wjblanke @cmmarslender @wallentx @hoffmang9