Allow live-reload to be disabled from command line. #510
Conversation
|
@rjackson mind adding tests? |
|
LGTM otherwise |
|
In the interests of brevity and symmetry with other CLI tools, should the flag be |
| @@ -9,6 +9,10 @@ exports.start = function(options) { | |||
| var leek = this.leek; | |||
| var ui = this.ui; | |||
|
|
|||
| if (options.disableLiveReload) { | |||
| return false; | |||
There was a problem hiding this comment.
return Promise.reject() for consistency
There was a problem hiding this comment.
Since we are calling Promise.all in the server command, returning Promise.reject would kill the expressServer (since Promise.all will return as soon as the first promise rejects).
I will use Promise.resolve as this isn't a failure case (since the user opted out).
|
@twokul I don't think that adding a test is necessary since we're planning to drop LiveReload in favor of a script that gets injected into the page. (No browser plugin needed) |
|
@MajorBreakfast when is it going to happen? if it's gonna be a part of |
|
@twokul Dunno. How would the test for this look like? It's probably a lot longer than the |
|
Updated to use |
|
Now with tests. |
| @@ -9,6 +9,10 @@ exports.start = function(options) { | |||
| var leek = this.leek; | |||
| var ui = this.ui; | |||
|
|
|||
| if (!options.liveReload) { | |||
| return Promise.resolve('live-reload is disabled'); | |||
There was a problem hiding this comment.
Looks exotic. But if it makes @twokul happy...
Enabled: resolves to undefined
Disabled: resolves to a String ^^' yah...
Allow live-reload to be disabled from command line.
Fixes #509.