-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
fix(server): fix setupExitSignals usage #2181
Conversation
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.
Not sure, how it is solve problem
Codecov Report
@@ Coverage Diff @@
## master #2181 +/- ##
==========================================
+ Coverage 96.04% 96.07% +0.02%
==========================================
Files 33 34 +1
Lines 1213 1222 +9
Branches 345 346 +1
==========================================
+ Hits 1165 1174 +9
Misses 47 47
Partials 1 1
Continue to review full report at Codecov.
|
Before, we were just passing undefined into the function. The function could not see when the variable was set to something else outside of the function. Here, I am passing an object into the function. The |
@Loonride can we add tests fir this case? i am tired, stupid question above 😄 |
Yes I will do that soon.
no problem, it is definitely not an obvious difference |
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.
Just fix lint problem, good job!
@evilebottnawi It looks like husky is not working for me. Usually works for me on Windows 7, but I'm on Ubuntu 16.04 right now. Possibly just a bad |
don't worry, i will fix commit message before merge (I often correct them for a better changelog), anyway using good commit message help me to do it 😄 |
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.
/cc @hiroppy
Thanks |
For Bugs and Features; did you add new tests?
No, should I add tests for
setupExitSignals
?Motivation / Use-Case
Fixes problem with
setupExitSignals
mentioned in: #1479An alternative to this approach is simply to get rid of
setupExitSignals
, but I think it's better to keep the helper and just change the function a bit.Breaking Changes
Altered
setupExitSignals
function API.Additional Info