-
Notifications
You must be signed in to change notification settings - Fork 639
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
Removing Forever dependency #971
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.
Environment for child_process
bin/ungit
Outdated
}); | ||
var child = child_process.spawn('node', | ||
[path.join(__dirname, '..', 'src', 'server.js')].concat(process.argv.slice(2)), | ||
{ cwd: path.join(process.cwd(), '..') } |
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.
What about the env
setting?
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.
It didn't seems to be respected as apparent by #959. I may leave it in with a note though...
var child = new (forever.Monitor)(path.join(__dirname, '..', 'src', 'server.js'), { | ||
silent: false, | ||
minUptime: 2000, | ||
max: config.maxNAutoRestartOnCrash, |
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.
Is maxNAutoRestartOnCrash
used anywhere else?
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 really... From my tests forever monitor actually never kicked in and process didn't die during errors. Errors seems to be captured and handled so..
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.
Ok, so that means the config setting can be removed.
CHANGELOG.md
Outdated
@@ -3,7 +3,9 @@ All notable changes to this project will be documented in this file. | |||
This project adheres to [Semantic Versioning](http://semver.org/). | |||
Use the following format for additions: ` - VERSION: [feature/patch (if applicable)] Short description of change. Links to relevant issues/PRs.` | |||
|
|||
- 1.1.31: Bump dependencies | |||
- 1.1.31: |
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.
Also bump version in package.json
.
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
I will try to refactor it to es6 without async.