Skip to content
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

Move away from using enums for Deno.Signal API #11900

Closed
bartlomieju opened this issue Sep 2, 2021 · 0 comments · Fixed by #11909
Closed

Move away from using enums for Deno.Signal API #11900

bartlomieju opened this issue Sep 2, 2021 · 0 comments · Fixed by #11909
Labels
feat new feature (which has been agreed to/accepted) public API related to "Deno" namespace in JS

Comments

@bartlomieju
Copy link
Member

Currently we got two huge enums for Signal definition:

enum LinuxSignal {
SIGHUP = 1,
SIGINT = 2,
SIGQUIT = 3,
SIGILL = 4,
SIGTRAP = 5,
SIGABRT = 6,
SIGBUS = 7,
SIGFPE = 8,
SIGKILL = 9,
SIGUSR1 = 10,
SIGSEGV = 11,
SIGUSR2 = 12,
SIGPIPE = 13,
SIGALRM = 14,
SIGTERM = 15,
SIGSTKFLT = 16,
SIGCHLD = 17,
SIGCONT = 18,
SIGSTOP = 19,
SIGTSTP = 20,
SIGTTIN = 21,
SIGTTOU = 22,
SIGURG = 23,
SIGXCPU = 24,
SIGXFSZ = 25,
SIGVTALRM = 26,
SIGPROF = 27,
SIGWINCH = 28,
SIGIO = 29,
SIGPWR = 30,
SIGSYS = 31,
}
enum MacOSSignal {
SIGHUP = 1,
SIGINT = 2,
SIGQUIT = 3,
SIGILL = 4,
SIGTRAP = 5,
SIGABRT = 6,
SIGEMT = 7,
SIGFPE = 8,
SIGKILL = 9,
SIGBUS = 10,
SIGSEGV = 11,
SIGSYS = 12,
SIGPIPE = 13,
SIGALRM = 14,
SIGTERM = 15,
SIGURG = 16,
SIGSTOP = 17,
SIGTSTP = 18,
SIGCONT = 19,
SIGCHLD = 20,
SIGTTIN = 21,
SIGTTOU = 22,
SIGIO = 23,
SIGXCPU = 24,
SIGXFSZ = 25,
SIGVTALRM = 26,
SIGPROF = 27,
SIGWINCH = 28,
SIGINFO = 29,
SIGUSR1 = 30,
SIGUSR2 = 31,
}
/** **UNSTABLE**: Further changes required to make platform independent.
*
* Signals numbers. This is platform dependent. */
export const Signal: typeof MacOSSignal | typeof LinuxSignal;

This is bad, as there are different enums depending on the OS.

After initial discussion between the core team we decided we should just use strings for signals.

This is prerequisite for #11757

@bartlomieju bartlomieju added feat new feature (which has been agreed to/accepted) public API related to "Deno" namespace in JS labels Sep 2, 2021
ry added a commit to ry/deno that referenced this issue Sep 3, 2021
@ry ry closed this as completed in #11909 Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat new feature (which has been agreed to/accepted) public API related to "Deno" namespace in JS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant