Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

Support Graceful Restarts #2716

Closed
wants to merge 1 commit into from
Closed

Support Graceful Restarts #2716

wants to merge 1 commit into from

Conversation

scien
Copy link
Contributor

@scien scien commented Aug 14, 2012

isaacs/npm#1999

npm restart currently runs prestop, stop, poststop, restart, prestart, start, poststart. According to the docs, and the original intention of the feature, a restart script should be able to be provided so the application can choose to restart their service gracefully (see isaacs/npm#184) instead of issueing a full stop/start.

Note: npm restart will run the stop and start scripts if no restart script is provided.

A full stop/start should only happen if a restart script is not provided.

@ralt
Copy link
Contributor

ralt commented Mar 16, 2013

Looks like the commit log should be "Don't run restart if it doesn't exist", shouldn't it? I'm confused between your description and the code in the commit.

@domenic
Copy link
Contributor

domenic commented Mar 18, 2013

No, the commit message makes sense. LGTM, @isaacs?

@luk-
Copy link
Contributor

luk- commented Mar 18, 2013

LGTM

@eliasgs
Copy link

eliasgs commented Nov 12, 2013

Why isn't this merged?

@luk-
Copy link
Contributor

luk- commented Dec 3, 2013

I think we can probably merge this if someone wants to update the docs for it as well. If not, I can tomorrow or sometime this week.

@timoxley
Copy link
Contributor

timoxley commented Jan 7, 2014

@scien want to add "npm restart will run the stop and start scripts if no restart script is provided." to the docs?

#4430

@eliasgs
Copy link

eliasgs commented Jan 7, 2014

The docs says:
This runs a package's "restart" script, if one was provided. Otherwise it runs package's "stop" script, if one was provided, and then the "start" script.
It seems to sufficiently describe the functionality, if this pull request gets merged?

I think this should just be merged already:)

@domenic
Copy link
Contributor

domenic commented Jan 11, 2014

This needs a test before it can be merged.

@othiym23
Copy link
Contributor

This has been waiting for a test without getting one for over a year. Closing as abandoned. Thanks for taking the time to put it together, @scien, and sorry we let it languish for so long without providing feedback. If you want to take another stab at this, a new version, with tests, would be welcome.

@othiym23 othiym23 closed this Feb 27, 2015
@othiym23 othiym23 removed the review label Feb 27, 2015
othiym23 pushed a commit that referenced this pull request Feb 27, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants