Skip to content

refactor: pass context.TODO() by parameter #3877

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

Merged
merged 1 commit into from
Aug 21, 2025

Conversation

zyfy29
Copy link
Contributor

@zyfy29 zyfy29 commented Aug 20, 2025

close #3874

@Copilot Copilot AI review requested due to automatic review settings August 20, 2025 07:52
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the codebase to improve context management by passing context.Context as a parameter instead of using context.TODO() locally within functions. This addresses issue #3874 and follows Go best practices for context propagation.

  • Updates function signatures to accept context.Context as the first parameter
  • Removes local context.TODO() calls and imports where no longer needed
  • Preserves existing functionality by passing context.TODO() at call sites where context is not available

Reviewed Changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/windows/process_windows.go Updated GetProcessCommandLine to accept context parameter
pkg/sysprof/sysprof_darwin.go Updated SystemProfiler to accept context parameter
pkg/store/instance_windows.go Updated GetSSHAddress to accept context parameter
pkg/sshutil/sshutil_test.go Updated test to pass context.TODO() to DefaultPubKeys
pkg/sshutil/sshutil.go Updated multiple functions to accept context parameter
pkg/qemuimgutil/qemuimgutil.go Updated utility functions to accept context parameter
pkg/osutil/user.go Updated call function to accept context parameter
pkg/osutil/osutil_unix.go Updated Sysctl to accept context parameter
pkg/osutil/machineid.go Updated machineID to accept context parameter
pkg/networks/sudoers.go Updated passwordLessSudo to accept context parameter
pkg/networks/reconcile/reconcile.go Updated sudo function to accept context parameter
pkg/limayaml/defaults_test.go Updated test calls to pass context.TODO()
pkg/limayaml/defaults.go Updated WindowsSubsystemPath call to pass context.TODO()
pkg/ioutilx/ioutilx.go Updated path conversion functions to accept context parameter
pkg/hostagent/mount.go Updated WindowsSubsystemPath call to pass context.TODO()
pkg/hostagent/dns/dns.go Updated handleQuery to accept context parameter
pkg/guestagent/iptables/iptables.go Updated checkPortsOpen to accept context parameter
pkg/editutil/editutil.go Updated OpenEditor to accept context parameter
pkg/driver/vz/vm_darwin.go Updated DialQemu calls to pass context.TODO()
pkg/driver/vz/network_darwin_test.go Updated test to pass context.TODO() to DialQemu
pkg/driver/vz/network_darwin.go Updated DialQemu to accept context parameter
pkg/driver/qemu/qemu_driver.go Updated FindVirtiofsd call to pass context.TODO()
pkg/driver/qemu/qemu.go Updated multiple utility functions to accept context parameter
pkg/driver/qemu/entitlementutil/entitlementutil.go Updated signing functions to accept context parameter
pkg/driver/external/server/server.go Updated Serve to accept context parameter
pkg/cidata/cidata.go Updated DefaultPubKeys call to pass context.TODO()
pkg/autostart/autostart.go Updated enableDisableService to accept context parameter
cmd/limactl/shell.go Updated path conversion calls to pass context.TODO()
cmd/limactl/edit.go Updated OpenEditor call to pass context.TODO()
cmd/limactl/copy.go Updated WindowsSubsystemPath call to pass context.TODO()
cmd/lima-driver-wsl2/main_windows.go Updated Serve call to pass context.TODO()
cmd/lima-driver-vz/main_darwin.go Updated Serve call to pass context.TODO()
cmd/lima-driver-qemu/main.go Updated Serve call to pass context.TODO()

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@@ -100,7 +101,7 @@ func editAction(cmd *cobra.Command, args []string) error {
hdr += "# and an empty file will abort the edit.\n"
hdr += "\n"
hdr += editutil.GenerateEditorWarningHeader()
yBytes, err = editutil.OpenEditor(yContent, hdr)
yBytes, err = editutil.OpenEditor(context.TODO(), yContent, hdr)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
yBytes, err = editutil.OpenEditor(context.TODO(), yContent, hdr)
yBytes, err = editutil.OpenEditor(ctx, yContent, hdr)

w/ ctx := cmd.Context()

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks, but this doesn't follow what I meant in #3874

@zyfy29
Copy link
Contributor Author

zyfy29 commented Aug 20, 2025

Oh, I understand what your mean. The ctx should be passed over from the deamon or command context, is it right? I will update my work later.

@zyfy29 zyfy29 force-pushed the parse-todo-context-by-parameter branch from 92a859c to 78ab49d Compare August 20, 2025 13:10
@zyfy29
Copy link
Contributor Author

zyfy29 commented Aug 20, 2025

Windows tests QEMU successed at the last time, maybe we can try it again.

@zyfy29 zyfy29 requested a review from AkihiroSuda August 20, 2025 15:04
@AkihiroSuda
Copy link
Member

Please squash the commits
https://lima-vm.io/docs/dev/git/#squashing-commits

@AkihiroSuda AkihiroSuda added this to the v2.0.0 milestone Aug 20, 2025
@zyfy29 zyfy29 force-pushed the parse-todo-context-by-parameter branch from c2686ea to aa5a42c Compare August 21, 2025 00:46
@zyfy29 zyfy29 requested a review from AkihiroSuda August 21, 2025 01:29
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda merged commit 1daec3a into lima-vm:master Aug 21, 2025
62 of 63 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.

Remove context.TODO()
2 participants