-
Notifications
You must be signed in to change notification settings - Fork 28
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
electron@19 #159
electron@19 #159
Conversation
On it. |
399f406
to
515d898
Compare
runs-on: ${{ matrix.os }} | ||
steps: | ||
- uses: actions/checkout@v3 |
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.
avoid changes like this. See, me currently close-to-sleep, superdizzy. Possibly an autoformatter did it?
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.
This is a git checkout action. I don't see a problem with the bump. If you insist I will downgrade
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.
This is a benefit (enhanced caching) of using newer versions:
Run actions/setup-node@v4
Found in cache @ /opt/hostedtoolcache/node/20.18.1/x6[4](https://github.com/deepnest-io/Deepnest/actions/runs/12189545706/job/34004910615?pr=159#step:4:4)
Environment details
@@ -8,7 +8,6 @@ | |||
/bin | |||
/minkowski | |||
/deepnest-*win32-x64 | |||
package-lock.json |
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.
This is the same as your comment against using ubuntu-latest. Without package.json you are trusting the ci machine and npm/node to do an exact install of node modules without actually specifying that is what you want. Extremely undesirable. That is the whole reason of the cmd npm ci
- install in accordance to lock file on ci.
@@ -8,7 +8,6 @@ | |||
/bin | |||
/minkowski | |||
/deepnest-*win32-x64 | |||
package-lock.json |
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.
This is the same as your comment against using ubuntu-latest. Without package.json you are trusting the ci machine and npm/node to do an exact install of node modules without actually specifying that is what you want. Extremely undesirable. That is the whole reason of the cmd npm ci
- install in accordance to lock file on ci.
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.
I am done
Node is working pretty reliably, see this server impl: Screen.Recording.2024-12-06.at.14.02.09.mov |
See commits for details
Removing electron-settings means loss of the user's local settings file unless we know where to look for it, and we do. I removed it because it didn't play nicely with the new remote IIRC.