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

Update electron-builder.js #52

Merged
merged 4 commits into from
Oct 18, 2023
Merged

Conversation

kpanuragh
Copy link
Contributor

@kpanuragh kpanuragh commented Jul 28, 2023

  • Added platformType variable to store the current platform type
  • Updated isWindows, isLinux, and isDarwin variables to use platformType instead of process.argv.includes()
  • Updated phpBinaryFilename variable to use 'php' as the default value if isWindows is true

- Added platformType variable to store the current platform type
- Updated isWindows, isLinux, and isDarwin variables to use platformType instead of process.argv.includes()
- Updated targetOs variable to use 'mac' as the default value
- Updated phpBinaryFilename variable to use 'php' as the default value if isWindows is true
@chrisreedio
Copy link
Contributor

Please see my PR as I handle most if not all of this. #47

Please note, the default targetOs should not be 'mac' as the develop command should run natively on the same OS as the developer's OS.

The build scripts pass in the target build OS which then overrides the current platform.

@kpanuragh
Copy link
Contributor Author

@chrisreedio updated default os

@mpociot
Copy link
Member

mpociot commented Jul 31, 2023

Thank you for the PR @kpanuragh
I'll wait for @ShaneShipston to update his PR before getting back to your PR as he mentioned that some of this is already covered in it.

@kpanuragh
Copy link
Contributor Author

kpanuragh commented Aug 2, 2023

@mpociot added additional condition checking for platform detection in mac and windows resolved conflict with #47

@mpociot mpociot merged commit a5404c8 into NativePHP:main Oct 18, 2023
1 check passed
@mpociot
Copy link
Member

mpociot commented Oct 18, 2023

Thank you!

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.

3 participants