This repository has been archived by the owner on Feb 16, 2024. It is now read-only.
Allow custom electron path #311
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
asticode
suggested changes
Feb 20, 2021
paths.go
Outdated
| @@ -64,7 +64,7 @@ func newPaths(os, arch string, o Options) (p *Paths, err error) { | |||
| p.electronDownloadSrc = ElectronDownloadSrc(os, arch, o.VersionElectron) | |||
There was a problem hiding this comment.
I'd rather
- you put lines 63 --> 67 in a
if o.CustomElectronPath == "" - and in this function, the
elseis directlyp.appExecutable = o.CustomElectronPath,
Otherwise this won't work
paths.go
Outdated
| @@ -64,7 +64,7 @@ func newPaths(os, arch string, o Options) (p *Paths, err error) { | |||
| p.electronDownloadSrc = ElectronDownloadSrc(os, arch, o.VersionElectron) | |||
| p.electronDownloadDst = filepath.Join(p.vendorDirectory, fmt.Sprintf("electron-%s-%s-v%s.zip", os, arch, o.VersionElectron)) | |||
| p.electronUnzipSrc = p.electronDownloadDst | |||
| p.initAppExecutable(os, o.AppName) | |||
| p.initAppExecutable(os, o.CustomElectronPath, o.AppName) | |||
There was a problem hiding this comment.
No need to inject o.CustomElectronPath anymore
paths.go
Outdated
| case "windows": | ||
| p.appExecutable = filepath.Join(p.electronDirectory, "electron.exe") | ||
| } else { | ||
| p.appExecutable = customElectronPath; |
There was a problem hiding this comment.
lol habits die hard ;)
provisioner.go
Outdated
| @@ -151,6 +151,8 @@ func (p *defaultProvisioner) provisionAstilectron(ctx context.Context, paths Pat | |||
|
|
|||
| // provisionElectron provisions electron | |||
| func (p *defaultProvisioner) provisionElectron(ctx context.Context, paths Paths, s ProvisionStatus, appName, os, arch, versionElectron string) error { | |||
| if paths.ElectronUnzipSrc() == "" { return nil } | |||
There was a problem hiding this comment.
With GO formatting, this needs new lines:
if paths.ElectronUnzipSrc() == "" {
return nil
}
provisioner.go
Outdated
| @@ -151,6 +151,8 @@ func (p *defaultProvisioner) provisionAstilectron(ctx context.Context, paths Pat | |||
|
|
|||
| // provisionElectron provisions electron | |||
| func (p *defaultProvisioner) provisionElectron(ctx context.Context, paths Paths, s ProvisionStatus, appName, os, arch, versionElectron string) error { | |||
| if paths.ElectronUnzipSrc() == "" { return nil } | |||
|
|
|||
raboof
force-pushed
the
allow-custom-electron-path
branch
from
0b34d5c to
5484f63
Compare
asticode
suggested changes
Feb 20, 2021
paths.go
Outdated
| @@ -144,7 +148,6 @@ func ElectronDownloadSrc(os, arch, versionElectron string) string { | |||
| return fmt.Sprintf("https://github.com/electron/electron/releases/download/v%s/electron-v%s-%s-%s.zip", versionElectron, versionElectron, o, a) | |||
| } | |||
|
|
|||
| // initAppExecutable initializes the app executable path | |||
|
Oops that's an unintentional leftover from the now-rolled-back change. Will add it back - though perhaps it's an indication it should be renamed 'appExecutablePath'?
…On 20 February 2021 09:44:46 CET, Quentin Renard ***@***.***> wrote:
@asticode requested changes on this pull request.
> @@ -144,7 +148,6 @@ func ElectronDownloadSrc(os, arch, versionElectron string) string {
return fmt.Sprintf("https://github.com/electron/electron/releases/download/v%s/electron-v%s-%s-%s.zip", versionElectron, versionElectron, o, a)
}
-// initAppExecutable initializes the app executable path
Can you keep this comment?
--
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#311 (review)
|
|
no worries. I think Once you've made the change, can you test running the example in your NixOS and let me know whether this works properly? |
|
Yes, that works!
…On 20 February 2021 11:05:43 CET, Quentin Renard ***@***.***> wrote:
no worries. I think `initAppExecutable` is fine for now.
Once you've made the change, can you test running the example in your NixOS and let me know whether this works properly?
--
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#311 (comment)
|
raboof
force-pushed
the
allow-custom-electron-path
branch
from
5484f63 to
560b52e
Compare
asticode
suggested changes
Feb 21, 2021
example/main.go
Outdated
| @@ -14,8 +14,9 @@ func main() { | |||
|
|
|||
| // Create astilectron | |||
| a, err := astilectron.New(l, astilectron.Options{ | |||
| AppName: "Test", | |||
| BaseDirectoryPath: "example", | |||
| //CustomElectronPath: "/nix/store/srxbsf62sapfi2vginqshhicbqgzsnsx-electron-11.2.3/bin/electron", | |||
|
OK great, just remove the last comment and I'll merge the PR |
raboof
force-pushed
the
allow-custom-electron-path
branch
from
560b52e to
38e9da9
Compare
This is useful on systems (like NixOS) where the Electron downloaded by go-astilectron does not work out of the box, but it is possible to install a working Electron separately.
raboof
force-pushed
the
allow-custom-electron-path
branch
from
38e9da9 to
0b464bc
Compare
|
Cheers |
|
thanks for the detailed directions/feedback! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is useful on systems (like NixOS) where the Electron downloaded
by go-astilectron does not work out of the box, but it is possible to
install a working Electron separately.
Refs #309