Skip to content

Conversation

ksss
Copy link
Contributor

@ksss ksss commented Sep 29, 2016

Signal.signame(0).should == "EXIT"
Signal.signame(1).should == "HUP"
Signal.signame(2).should == "INT"
end
Copy link
Member

Choose a reason for hiding this comment

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

You should wrap this spec with:

platform_is_not :windows do

as I think these numbers are POSIX-specific.

Copy link
Member

Choose a reason for hiding this comment

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

It's also possible to use Signal.list to avoid hard-coding numbers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to depend on Signal.list.
But, It's not strong opinion.
I'll use Signal.list.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it's a trade-off. If the numbers are portable on POSIX platforms, it's OK to use hardcoded numbers.

@eregon
Copy link
Member

eregon commented Sep 29, 2016

Looks good!
Can you add a guard so it also passes the Windows CI?

@ksss ksss force-pushed the signame branch 2 times, most recently from 534af37 to 677bb28 Compare September 30, 2016 01:04
@ksss
Copy link
Contributor Author

ksss commented Sep 30, 2016

Finally, I wrote just Signal.signame(0).should == "EXIT".
0 is a special signal number on CRuby.
It works on all environment.

@eregon eregon merged commit cdd885f into ruby:master Sep 30, 2016
@eregon
Copy link
Member

eregon commented Sep 30, 2016

Thank you for your contribution!

@ksss ksss deleted the signame branch September 30, 2016 08:15
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.

2 participants