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

Differentiate signal handling for windows #382

Merged
merged 2 commits into from
Nov 22, 2016
Merged

Conversation

ColinSullivan1
Copy link
Member

@ColinSullivan1 ColinSullivan1 commented Nov 22, 2016

Windows has limited support for signals, and does not define syscall.SIGUSR1. Log rotation will be handled differently in windows.

  • Add signal.go for all non-windows builds
  • Add signal_windows.go for windows builds.

Today, windows looks to be the only platform that does not have syscall.SIGUSR1 defined.

Resolves #381

Windows has limited support for signals, and does not define syscall.SIGUSR1.  Log rotation will be handled differently in windows.

* Add signal.go for all non-windows builds
* Add signal_windows.go for windows builds.

Today, windows looks to be the only platform that does not have syscall.SIGUSR1 defined.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 89.249% when pulling ccca711 on win-signals into ee70a09 on master.

}
c := make(chan os.Signal, 1)

signal.Notify(c, syscall.SIGINT)
Copy link
Member

Choose a reason for hiding this comment

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

We could revert back to os.Interrupt here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will change.

go func() {
for sig := range c {
Debugf("Trapped %q signal", sig)
switch sig {
Copy link
Member

Choose a reason for hiding this comment

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

Only one sig, so no need to switch etc,

Copy link
Member Author

@ColinSullivan1 ColinSullivan1 Nov 22, 2016

Choose a reason for hiding this comment

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

Will change.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 89.358% when pulling e9dc5fa on win-signals into ee70a09 on master.

@derekcollison derekcollison merged commit e8dcb33 into master Nov 22, 2016
@derekcollison derekcollison deleted the win-signals branch November 22, 2016 23:28
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.

3 participants