Skip to content

Conversation

egor-zalenski
Copy link
Contributor

#RI-4265 - Enhance the window opening in Electron

Copy link
Collaborator

@ArtemHoruzhenko ArtemHoruzhenko left a comment

Choose a reason for hiding this comment

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

I dind't look to ui/* part
In general looks nice but have few questions to address before approve

@@ -31,9 +30,6 @@ export default merge(mainProdConfig, {
APP_VERSION: version,
AWS_BUCKET_NAME: 'AWS_BUCKET_NAME' in process.env ? process.env.AWS_BUCKET_NAME : '',
SEGMENT_WRITE_KEY: 'SEGMENT_WRITE_KEY' in process.env ? process.env.SEGMENT_WRITE_KEY : 'SOURCE_WRITE_KEY',
CONNECTIONS_TIMEOUT_DEFAULT: 'CONNECTIONS_TIMEOUT_DEFAULT' in process.env
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you remove this env?


contextBridge.exposeInMainWorld('electron', electronHandler);

contextBridge.exposeInMainWorld('ENV_VARS', {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't like to export env variables. Let's have electron.config object with everything needed. What do you think?

import path from 'path';

export function resolveHtmlPath(htmlFileName: string) {
if (process.env.NODE_ENV === 'development') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I really do not understand why somewhere we are using envVars somewhere process.env and what was the goal

Copy link
Collaborator

@ArtemHoruzhenko ArtemHoruzhenko left a comment

Choose a reason for hiding this comment

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

Approve with agreement that not resolved comments will be addressed in another PR #2182

@egor-zalenski egor-zalenski changed the base branch from main to feature/RI-2850_Electron_improvements June 12, 2023 12:31
@egor-zalenski egor-zalenski merged commit 49cb3f4 into feature/RI-2850_Electron_improvements Jun 12, 2023
@egor-zalenski egor-zalenski deleted the build/feature/RI-4265_Enchance_opening_window_Electron branch June 12, 2023 12:32
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.

2 participants