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

Stop using @electron/remote to obtain app.getPath() #4078

Merged
merged 7 commits into from
Oct 28, 2021
Merged

Conversation

Nokel81
Copy link
Collaborator

@Nokel81 Nokel81 commented Oct 19, 2021

Signed-off-by: Sebastian Malton sebastian@malton.name

Fixes a bug where, for some reason, remote.app throws an error (during early lifetime of a frame). This fixes the issue by instead initializing the whole set in bootstrap() which is called far enough along that it shouldn't observe that behaviour.

The changes to createStorage are required because they were the initial error case, they now wait for bootstrap to call AppPaths.init() and then they will initialize themselves.

@Nokel81 Nokel81 added the bug Something isn't working label Oct 19, 2021
@Nokel81 Nokel81 requested a review from a team as a code owner October 19, 2021 18:29
@Nokel81 Nokel81 requested review from jweak and jim-docker and removed request for a team October 19, 2021 18:29
@Nokel81 Nokel81 changed the title Remove ClusterStore.removedClusters as it is not correctly used Stop using remote to obtain app.getPath() Oct 21, 2021
@Nokel81 Nokel81 changed the title Stop using remote to obtain app.getPath() Stop using @electron/remote to obtain app.getPath() Oct 21, 2021
@jakolehm jakolehm added this to the 5.3.0 milestone Oct 22, 2021
src/common/app-paths.ts Outdated Show resolved Hide resolved
@Nokel81 Nokel81 requested a review from jweak October 26, 2021 14:52
}
}

private static initMain(): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

This gets called for sure before initRenderer()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I hope so, it gets called in the main process whereas initRenderer gets called in each frame.

src/main/kubectl.ts Outdated Show resolved Hide resolved
src/renderer/utils/createStorage.ts Show resolved Hide resolved
src/renderer/utils/createStorage.ts Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

- Initialize entire map in bootstrap()

- Updated unit tests

Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
@github-actions
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

Signed-off-by: Sebastian Malton <sebastian@malton.name>
@Nokel81
Copy link
Collaborator Author

Nokel81 commented Oct 27, 2021

@jim-docker PTALA

Signed-off-by: Sebastian Malton <sebastian@malton.name>
@Nokel81 Nokel81 merged commit f297407 into master Oct 28, 2021
@Nokel81 Nokel81 deleted the remove-getPath branch October 28, 2021 01:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants