Skip to content

Remove signal notify boilerplate for now#434

Closed
pfleidi wants to merge 4 commits intomainfrom
signal-notifycontext
Closed

Remove signal notify boilerplate for now#434
pfleidi wants to merge 4 commits intomainfrom
signal-notifycontext

Conversation

@pfleidi
Copy link

@pfleidi pfleidi commented Feb 19, 2026

Entire-Checkpoint: 4d7e19fbfd1e

Summary

This pull request simplifies the startup and shutdown logic in the cmd/entire/main.go file by removing interrupt signal handling and the use of context cancellation. The main function now directly executes the root command and handles errors without additional cleanup logic.

This is mostly a stopgap solution for now until we have added consistent plumbing to forward the root context to the places using it.

Test plan

  • mise run fmt && mise run lint && mise run test:ci passes
  • Manual test: run a command and hit Ctrl+C to confirm graceful shutdown still works

Copilot AI review requested due to automatic review settings February 19, 2026 21:33
@pfleidi pfleidi requested a review from a team as a code owner February 19, 2026 21:33
@pfleidi pfleidi changed the title Use signal.NotifyContext() instead of signal.Notify() boilerplat Use signal.NotifyContext() instead of signal.Notify() boilerplate Feb 19, 2026
Copy link
Contributor

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 simplifies CLI shutdown handling by replacing the manual signal.Notify() + goroutine cancellation boilerplate with Go’s signal.NotifyContext(), keeping the root command execution cancelable via context.

Changes:

  • Replace manual signal channel + goroutine cancellation with signal.NotifyContext(...)
  • Keep context cancellation wired into ExecuteContext for graceful shutdown on Ctrl+C / SIGTERM

No command uses cmd.Context(), so the signal.NotifyContext setup
was dead code. Switch from ExecuteContext to plain Execute.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Entire-Checkpoint: 579b31e91172
@pfleidi pfleidi changed the title Use signal.NotifyContext() instead of signal.Notify() boilerplate Remove signal notify boilerplate for now Feb 20, 2026
@pfleidi
Copy link
Author

pfleidi commented Feb 20, 2026

I'm closing this PR in favor of adding plumbing to hand down the root context to the places that need it. I'll open a new PR for the new changes.

@pfleidi pfleidi closed this Feb 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments