Skip to content

Conversation

@gwleuverink
Copy link
Collaborator

@gwleuverink gwleuverink commented Oct 9, 2025

Fixed all conflicts with base branch in PR #3

@gwleuverink gwleuverink force-pushed the merge/migrate-to-bifrost branch from 943f58d to c436565 Compare October 9, 2025 20:26
@gwleuverink
Copy link
Collaborator Author

@SRWieZ I've update your original PR to fit within the new structure.

Also made a small change in how the ManagesEnvFile trait is used. Thought it is a bit more deterministic to always use it through the builder class. What do you think?

@gwleuverink
Copy link
Collaborator Author

The static analysis workflow is fixed in #6

If we merge that one first & then update upstream on this branch everything should be green


private function prepareBundlePath(): string
{
$buildDir = base_path('build');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@SRWieZ Something to consider:

The dist directory has moved to nativephp/electron/dist. Does it make sense to download the bundle inside there too? maybe in nativephp/electron/bundle?

Copy link
Member

Choose a reason for hiding this comment

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

if nativephp/electron/dist is where Electron binaries are after native:build, then it's good, it's easier if I only have one directory to check for Bifrost builds.

@gwleuverink gwleuverink requested a review from SRWieZ October 9, 2025 21:06
@SRWieZ
Copy link
Member

SRWieZ commented Oct 10, 2025

@SRWieZ I've update your original PR to fit within the new structure.

Also made a small change in how the ManagesEnvFile trait is used. Thought it is a bit more deterministic to always use it through the builder class. What do you think?

I don't know if I need the builder class in that case, it's just one command that helps you log in to Bifrost in the end.


Otherwise, PR looks good, you can merge it, and I will try it and fix it later.

@gwleuverink
Copy link
Collaborator Author

Sweet. I'll merge it 👌🏻

@gwleuverink gwleuverink merged commit 36c40c0 into main Oct 10, 2025
30 checks 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.

3 participants