-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
chore(signals): prefactor #9995
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Looks good, just a couple questions
edition = "2021" | ||
|
||
[dependencies] | ||
futures = "0.3.30" |
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.
Any particular reason we're using a different version for futures instead of the workspace root version?
#[cfg(windows)] | ||
const DEFAULT_SIGNAL: Signal = Signal::CtrlC; | ||
#[cfg(not(windows))] | ||
const DEFAULT_SIGNAL: Signal = Signal::Interrupt; |
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.
Nit, but we could do and avoid the conditional compilation
#[cfg(windows)] | |
const DEFAULT_SIGNAL: Signal = Signal::CtrlC; | |
#[cfg(not(windows))] | |
const DEFAULT_SIGNAL: Signal = Signal::Interrupt; | |
const DEFAULT_SIGNAL: Signal = if cfg!(windows) { Signal::CtrlC } else {Signal::Interrupt }; |
|
||
#[cfg(windows)] | ||
/// A listener for Windows Console Ctrl-C events | ||
pub fn get_signal() -> Result<impl Stream<Item = Option<Signal>>, std::io::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.
Minor detail, but maybe we should make this a custom error type so we can annotate the run::Error
type with #[from]
and avoid having to use map_err
when calling this?
Description
Prefactors:
PR that prefactors the signals code to prepare for:
turbo
There are no behavior changes in this PR. I highly suggest reviewing each commit on its own.
Testing Instructions
Existing unit tests.
Quick manual verification that existing
SIGINT
behavior remains the same: