-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
v2 [#3133] SingleInstanceData: Fill missing WD as is #3154
Conversation
WalkthroughThe recent update involves enhancing the Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChat with CodeRabbit Bot (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files selected for processing (2)
- v2/internal/frontend/desktop/linux/single_instance.go (1 hunks)
- v2/internal/frontend/desktop/windows/single_instance.go (1 hunks)
Additional comments: 2
v2/internal/frontend/desktop/linux/single_instance.go (1)
- 55-65: Verify if
os.Exit(1)
is the intended behavior after sending the message to the existing instance. Ensure that any necessary cleanup is performed before exiting.v2/internal/frontend/desktop/windows/single_instance.go (1)
- 51-57: Verify if
os.Exit(0)
is the intended behavior after sending the message to the existing instance. Ensure that any necessary cleanup is performed before exiting.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Thanks 🙏 Please could you add an entry to the changelog located at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (2)
- v2/internal/frontend/desktop/linux/single_instance.go (1 hunks)
- v2/internal/frontend/desktop/windows/single_instance.go (1 hunks)
Additional comments: 6
v2/internal/frontend/desktop/linux/single_instance.go (3)
58-62: The use of
os.Getwd()
to set the working directory is correct, but ensure that the working directory is always the intended value in all use cases, as it can vary depending on how the application is launched.66-66: Error logging for marshaling data is correctly implemented. Ensure that the log output is monitored and that these logs are stored securely, as they may contain sensitive information.
69-69: The use of
os.Exit(1)
is appropriate for terminating the second instance of the application. However, ensure that this behavior is documented and expected by the application's users, as it can be abrupt.v2/internal/frontend/desktop/windows/single_instance.go (3)
54-58: The use of
os.Getwd()
to set the working directory is correct, but ensure that the working directory is always the intended value in all use cases, as it can vary depending on how the application is launched.60-62: Error logging for marshaling data is correctly implemented. Ensure that the log output is monitored and that these logs are stored securely, as they may contain sensitive information.
65-65: The use of
os.Exit(0)
is appropriate for terminating the second instance of the application. However, ensure that this behavior is documented and expected by the application's users, as it can be abrupt.
if err != nil { | ||
log.Printf("Failed to get working directory: %v", err) | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proper error handling is implemented for the os.Getwd()
call. However, consider if returning early is the best approach, as it may prevent further necessary cleanup or logic that should execute regardless of this error.
if err != nil { | ||
log.Printf("Failed to get working directory: %v", err) | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proper error handling is implemented for the os.Getwd()
call. However, consider if returning early is the best approach, as it may prevent further necessary cleanup or logic that should execute regardless of this error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- v2/internal/frontend/desktop/linux/single_instance.go (2 hunks)
- v2/internal/frontend/desktop/windows/single_instance.go (2 hunks)
- website/src/pages/changelog.mdx (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- v2/internal/frontend/desktop/linux/single_instance.go
- v2/internal/frontend/desktop/windows/single_instance.go
Additional comments: 1
website/src/pages/changelog.mdx (1)
- 27-27: The changelog entry for PR v2 [#3133] SingleInstanceData: Fill missing WD as is #3154 is clear and accurately describes the fix implemented for the uninitialized
SecondInstanceData.WorkingDirectory
on Linux and Windows platforms. It succinctly captures the essence of the changes made and the platforms affected.
I skipped that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for this 🙏
@Vovan-VE sorry, that didn't looked at this earlier, holidays knocked me away from the laptop 😅 Thank you for fixing this! |
Description
Minimal fix for missing
WorkingDirectory
for linux and windows.Fixes #3133
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Tested locally in my wails app by replacing wails repo to local forked repo. Worked fine under linux and windows. Darwin was not changed and I cannot test it.
go.mod
app.go
linux
first instance output
windows
first instance output
Test Configuration
Still same as in #3133
Checklist:
website/src/pages/changelog.mdx
with details of this PRSummary by CodeRabbit
SecondInstanceData.WorkingDirectory
on Linux and Windows.