-
Notifications
You must be signed in to change notification settings - Fork 178
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(app-shell): Upgrade the Electron version to 21 #11537
Changes from 57 commits
6a8c032
937772b
3b0a6a4
4edee4e
3c2799d
37b4f54
3851764
89d08e0
aad418f
9acc8dd
5b6585c
4dcb753
3b02c12
be96a91
3266dfc
c61ce88
041010f
ffef72b
be3168a
325a940
837dd63
15b3103
b243eb2
b391edb
aa9e778
d37dbd5
7dcb1c5
b7a3bad
bad99af
cbd1909
ac7962b
daa05a2
77e093a
41cafa6
2a876f7
2e2b18f
e171130
25bd332
dbe4b2c
0779289
9ef2be0
bf5d093
76d4aa9
ad53dc5
abadf01
1fe5a9e
a86edf8
d24c12e
19d8add
77c99ce
d78c239
94f0b14
a655eb2
cf2c1a6
f9419e6
1769b81
406b3ca
ec0fd3c
dc74e60
2d9819b
722f5ab
6cc0589
d100a09
f0b36dc
97eb67c
19ffda9
44b889f
80ac648
3e9b1b0
13dad73
b21524d
93a2431
2b5078b
7c64ea8
0ddff98
64cbf7f
af51613
dc1990d
532cdca
1e1e2b1
41047e2
7966b66
c96b6b0
962a0f6
bfc81d4
5a70730
04fdc29
c5e4218
98a52b0
1b632c9
1804eba
40e0a66
7d9d4e2
5f59748
17f3ed4
2544e05
26da4ba
00994e8
5ca6fc9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,7 +57,7 @@ jobs: | |
- uses: 'actions/checkout@v3' | ||
- uses: 'actions/setup-node@v3' | ||
with: | ||
node-version: '14' | ||
node-version: '16' | ||
- name: 'install udev' | ||
run: sudo apt-get update && sudo apt-get install libudev-dev | ||
- name: 'set complex environment variables' | ||
|
@@ -93,18 +93,20 @@ jobs: | |
# to run cross-platform just like builds, might as well do them in the same job | ||
strategy: | ||
matrix: | ||
os: ['windows-2019', 'ubuntu-22.04', 'macos-latest'] | ||
os: ['windows-2022', 'ubuntu-22.04', 'macos-latest'] | ||
name: 'opentrons app backend unit tests and build' | ||
needs: ['js-unit-test'] | ||
runs-on: ${{ matrix.os }} | ||
steps: | ||
- uses: 'actions/checkout@v3' | ||
- uses: 'actions/setup-node@v3' | ||
with: | ||
node-version: '14' | ||
node-version: '16' | ||
- uses: actions/setup-python@v4 | ||
with: | ||
python-version: '3.10' | ||
- name: 'downgrade npm version' | ||
run: npm install -g npm@6 | ||
- name: 'install libudev' | ||
if: startsWith(matrix.os, 'ubuntu') | ||
run: sudo apt-get update && sudo apt-get install libudev-dev | ||
|
@@ -150,8 +152,14 @@ jobs: | |
APPLE_ID: ${{ secrets.OT_APP_APPLE_ID }} | ||
APPLE_ID_PASSWORD: ${{ secrets.OT_APP_APPLE_ID_PASSWORD }} | ||
HOST_PYTHON: python | ||
# TODO kj 10/17/2022 this run will be used make -C when all issues are solved. | ||
run: | | ||
make -C app-shell dist-${{ matrix.os }} | ||
if [ '${{ matrix.os}}' == 'ubuntu-22.04'] | ||
then | ||
make -C app-shell dist-${{ matrix.os }} | ||
else | ||
CPPFLAGS="-std=c++17" make -C app-shell dist-${{ matrix.os }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you remind me again why we need to explicitly set the CPP version? i think it was a specific dependency but i can't remember also, is there path forward to deleting this? i guess this is related to my first question There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it is to support node version 16.9 and higher which uses a v8 version that requires c++17 to compile. the path forward is for node-gyp, electron-gyp, etc to update defautls to use c++17 for that node version and higher and for native extension packages (our friend usb-detection) to use those node-gyp/electron-gyp versions; or for those native extension packages to put the flag in their gyp binding files There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually I have tried to use node v16.8.0 for a while but it causes another node-gyp issue on ubuntu-22.04 and decided to use switched to |
||
fi | ||
|
||
- if: github.event_name != 'pull_request' | ||
name: 'upload github artifact' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,7 +42,7 @@ jobs: | |
- uses: 'actions/checkout@v3' | ||
- uses: 'actions/setup-node@v3' | ||
with: | ||
node-version: '14' | ||
node-version: '16' | ||
- name: 'install udev for usb-detection' | ||
run: sudo apt-get update && sudo apt-get install libudev-dev | ||
- name: 'cache yarn cache' | ||
|
@@ -77,7 +77,7 @@ jobs: | |
- uses: 'actions/checkout@v3' | ||
- uses: 'actions/setup-node@v3' | ||
with: | ||
node-version: '14' | ||
node-version: '16' | ||
- name: 'install udev for usb-detection' | ||
run: sudo apt-get update && sudo apt-get install libudev-dev | ||
- name: 'cache yarn cache' | ||
|
@@ -110,7 +110,7 @@ jobs: | |
- uses: 'actions/checkout@v3' | ||
- uses: 'actions/setup-node@v3' | ||
with: | ||
node-version: '14' | ||
node-version: '16' | ||
- name: 'set complex environment variables' | ||
id: 'set-vars' | ||
uses: actions/[email protected] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,7 +45,7 @@ jobs: | |
- uses: 'actions/checkout@v3' | ||
- uses: 'actions/setup-node@v3' | ||
with: | ||
node-version: '14' | ||
node-version: '16' | ||
- name: 'set complex environment variables' | ||
id: 'set-vars' | ||
uses: actions/[email protected] | ||
|
@@ -70,7 +70,7 @@ jobs: | |
npm config set cache ${{ github.workspace }}/.npm-cache | ||
yarn config set cache-folder ${{ github.workspace }}/.yarn-cache | ||
make setup-js | ||
# Use the if to run all the lint checks even of some fail | ||
# Use the if to run all the lint checks even of some fail | ||
shell: bash | ||
- name: 'lint js' | ||
if: always() && steps.setup-js.outcome == 'success' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
14 | ||
16 |
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.
we can put this in an
env
block if we want. I'm surprised it's not needed on the other OSs.