-
Notifications
You must be signed in to change notification settings - Fork 2
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
fix: intel mac proxy #467
fix: intel mac proxy #467
Conversation
forge.config.ts
Outdated
let resources = [ | ||
'./resources/' + getPlatform() + '/' + getArch(), | ||
] | ||
|
||
if (getPlatform() === 'mac') { | ||
resources = [ | ||
'./resources/mac/arm64', | ||
'./resources/mac/x86_64', | ||
] | ||
} |
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.
Nit: I'm not a big fan of reassigning variables, so wanted to suggest an alternative:
function getPlatformSpecificResources() {
if (getPlatform() === 'mac') {
return ['./resources/mac/arm64', './resources/mac/x86_64']
}
return ['./resources/' + getPlatform() + '/' + getArch()]
}
const config: ForgeConfig = {
// ...
extraResource: [
// ...
...getPlatformSpecificResources()
]
}
forge.config.ts
Outdated
@@ -9,6 +9,17 @@ import { FusesPlugin } from '@electron-forge/plugin-fuses' | |||
import { FuseV1Options, FuseVersion } from '@electron/fuses' | |||
import { getPlatform, getArch } from './src/utils/electron' | |||
|
|||
let resources = [ | |||
'./resources/' + getPlatform() + '/' + getArch(), |
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.
Can we use path.join('./resources', getPlatform(), getArch())
?
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.
makes absolutely more sense :D
package.json
Outdated
@@ -1,7 +1,7 @@ | |||
{ | |||
"name": "k6-studio", | |||
"productName": "k6 Studio", | |||
"version": "0.12.0", | |||
"version": "0.13.0", |
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 doesn't seem right 👀
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 just for testing and will be reverted before the PR gets merged 🙇
This reverts commit e3a76c5.
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.
🍎
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.
LGTM
Description
Fix #460
Fix #486
The build for both mac architectures is done on an arm machine and that would make the packager packager only the
arm64
folder even on anx86_64
architecture, this PR fixes that by including both folders on mac with a small cost of71~MB
increase of application sizeHow to Test
Checklist
npm run lint
) and all checks pass.npm test
) and all tests pass.Screenshots (if appropriate):
Related PR(s)/Issue(s)