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

chore: update electron dependency + Node.js #2199

Merged
merged 4 commits into from
Sep 26, 2023
Merged

chore: update electron dependency + Node.js #2199

merged 4 commits into from
Sep 26, 2023

Conversation

kittaakos
Copy link
Contributor

@kittaakos kittaakos commented Aug 29, 2023

Motivation

Use Node.js 18+ and the latest Electron version for the build and runtime.

Change description

Other information

For reviewers:

  • The diff might seem overwhelming, but most is just text formatting. I suggest checking the commits individually to understand better how tiny this PR is.

Reviewer checklist

  • PR addresses a single concern.
  • The PR has no duplicates (please search among the Pull Requests before creating one)
  • PR title and description are properly filled.
  • Docs have been added / updated (for bug fixes / features)

@kittaakos kittaakos added type: enhancement Proposed improvement topic: infrastructure Related to project infrastructure topic: code Related to content of the project itself labels Aug 29, 2023
@kittaakos kittaakos self-assigned this Aug 29, 2023
@kittaakos kittaakos changed the title chore: update dependencies chore: update electron dependency + Node.js Aug 30, 2023
@kittaakos kittaakos marked this pull request as ready for review August 30, 2023 08:52
Copy link
Contributor

@per1234 per1234 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 update the Node version in these workflows:

.prettierignore Outdated Show resolved Hide resolved
.prettierrc.json Show resolved Hide resolved
Akos Kitta added 4 commits September 1, 2023 10:21
narrow the `resources` folder constraint in
the .gitignore

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
remove `msvs_version` npm config on win32
Ref: nodejs/node-gyp#2822

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
 - Update to `electron-builder@24.6.3`.
 - Fix obsolete electron security section in the development docs.

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
@@ -1,6 +1,8 @@
### Building and start the app from the sources on Ubuntu Linux

Tested and verified on Ubuntu 18.04.4. The source will be checked out to `~/dev/git/arduino-ide`.
Tested and verified on Ubuntu 22.04. The source will be checked out to `~/dev/git/arduino-ide`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Chicken-egg problem: the main still does not support Node.js 18+. Here is what should work. The git checkout part differs

#!/bin/bash -i

sudo apt update \
&& sudo apt install --no-install-recommends --yes \
  git \
  gcc \
  curl \
  make \
  python3 \
  pkg-config \
  libx11-dev \
  libxkbfile-dev \
  build-essential \
  libsecret-1-dev \
&& wget -qO- https://raw.githubusercontent.com/nvm-sh/nvm/v0.35.3/install.sh | bash \
&& source ~/.bashrc \
&& nvm install 18.17 \
&& nvm use 18.17 \
&& nvm alias default 18.17 \
&& curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | sudo apt-key add - \
&& echo "deb https://dl.yarnpkg.com/debian/ stable main" | sudo tee /etc/apt/sources.list.d/yarn.list \
&& sudo apt update && sudo apt install --no-install-recommends yarn \
&& mkdir -p ~/dev/git/ \
&& rm -rf ~/dev/git/arduino-ide \
&& git clone https://github.com/arduino/arduino-ide.git ~/dev/git/arduino-ide \
&& git -C ~/dev/git/arduino-ide checkout origin/update-infra -b update-infra \
&& yarn --cwd ~/dev/git/arduino-ide \
&& yarn --cwd ~/dev/git/arduino-ide/electron-app rebuild \
&& yarn --cwd ~/dev/git/arduino-ide build \
&& yarn --cwd ~/dev/git/arduino-ide start

@kittaakos
Copy link
Contributor Author

Please review. Thank you!

Copy link
Contributor

@davegarthsimpson davegarthsimpson left a comment

Choose a reason for hiding this comment

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

all makes sense, I do not see any issues.

@kittaakos kittaakos merged commit ed2d8ad into main Sep 26, 2023
31 checks passed
@kittaakos kittaakos deleted the update-infra branch September 26, 2023 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself topic: infrastructure Related to project infrastructure type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants