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

Linux extra condition for electron builder. #57

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

lotharthesavior
Copy link
Contributor

To begin with, thank you so much for your hard work. This is an a fantasticep for the PHP community!

This is not meant to be the final solution, but it solves the problem pointed out at this issue: https://github.com/NativePHP/electron/issues/55

After briefly checking the code, I cou't find any place to trigger the npm post-installation to have the option --linux. Due to that, an extra check for the platform was needed for the initial script to copy the binary from Linux folder.

The same fix might be needed for Windows.

@mpociot
Copy link
Member

mpociot commented Jul 29, 2023

Thank you very much for taking the time to send a PR.
I'd like to wait before emerging this for @chrisreedio and his work on #47 as I think this will already incorporate this change as well as other windows related changes.
I'll come back to this once #47 is ready to be merged

@arnolem
Copy link

arnolem commented Jul 30, 2023

Thanks @lotharthesavior.
It works perfectly for me on WSL/Ubuntu 22.04

@marcelo2605
Copy link

Thanks @lotharthesavior! Works on Ubuntu 22.04.2

@haiderlatitude
Copy link

Can anyone explain to me what exactly to do on Ubuntu. I couldn't understand.

@mpociot
Copy link
Member

mpociot commented Jul 31, 2023

As the Windows PR still is being worked on and the changeset is so small, let's get this merged in now 🤷

Thanks again!

@mpociot mpociot merged commit 6cbb27b into NativePHP:main Jul 31, 2023
1 check passed
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.

5 participants