Skip to content

Chore/upgrade-electron-17#4529

Merged
jackkav merged 14 commits intoKong:developfrom
jackkav:chore/upgrade-electron-15
Mar 7, 2022
Merged

Chore/upgrade-electron-17#4529
jackkav merged 14 commits intoKong:developfrom
jackkav:chore/upgrade-electron-15

Conversation

@jackkav
Copy link
Contributor

@jackkav jackkav commented Feb 27, 2022

TODO:

  • bump electron, node and node-libcurl fork
  • bump electron-builder and related deps
  • fix insomnia-components storybook
  • prettify/Date primitive in inso unit test missing .now()
  • update @types/node to v16
  • see what else breaks

changelog(Improvements): We've upgraded to Electron 17. While this change should not affect any behavior of the app, please reach out to us if you run into any problems so that we can assist.

@vercel vercel bot temporarily deployed to Preview February 27, 2022 12:10 Inactive
@jackkav jackkav force-pushed the chore/upgrade-electron-15 branch from 185b132 to 47d7ee6 Compare February 27, 2022 12:28
@vercel vercel bot temporarily deployed to Preview February 27, 2022 12:29 Inactive
@jackkav jackkav self-assigned this Feb 27, 2022
@jackkav jackkav force-pushed the chore/upgrade-electron-15 branch from 47d7ee6 to 1008791 Compare February 28, 2022 19:10
@vercel vercel bot temporarily deployed to Preview February 28, 2022 19:10 Inactive
@jackkav jackkav force-pushed the chore/upgrade-electron-15 branch from 1008791 to 96a93cd Compare February 28, 2022 19:22
@vercel vercel bot temporarily deployed to Preview February 28, 2022 21:43 Inactive
@vercel vercel bot temporarily deployed to Preview February 28, 2022 21:58 Inactive
@jackkav jackkav force-pushed the chore/upgrade-electron-15 branch from 828970a to 63afdc0 Compare March 3, 2022 14:10
@jackkav jackkav marked this pull request as ready for review March 3, 2022 17:21
@DMarby DMarby self-requested a review March 3, 2022 17:26
@jackkav jackkav changed the title Chore/upgrade-electron-15 Chore/upgrade-electron-17 Mar 4, 2022
console.log(`[build] npm: ${childProcess.spawnSync('npm', ['--version']).stdout}`.trim());
console.log(`[build] node: ${childProcess.spawnSync('node', ['--version']).stdout}`.trim());

if (process.version.indexOf('v14.') !== 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

out of scope, perhaps, but we can get this from the package.json.engines field (or the nvmrc or something like that). maybe we should in the future after this PR so this becomes parametric?

Copy link
Contributor

@DMarby DMarby Mar 4, 2022

Choose a reason for hiding this comment

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

I guess we can probably remove this entirely instead since we run the check-version.js script as part of bootstrap anyway, which does use .nvmrc?

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean... while we're on the topic, what's stopping us from using the engines package.json field?

I hit this problem all the time where I (with the wrong node version) run npm run clean, which succeeds, then run npm run bootstrap which fails, but then if I try to run npm run clean again, it fails again because now it can't find lerna because it just deleted it. In practice, this happens when I run the npm run hard-reset command, which I use many times per day every day. Having an engines field would mean the check would happen before npm run clean and thus the problem would go away. But anyway..

Copy link
Contributor

@gatzjames gatzjames left a comment

Choose a reason for hiding this comment

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

Looks great! 🚀

@jackkav jackkav enabled auto-merge (squash) March 7, 2022 12:58
@jackkav jackkav merged commit 1f05683 into Kong:develop Mar 7, 2022
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.

4 participants