forever.config Configstore init and usage fixes #1115
Conversation
Fixes: TypeError: forever.config.saveSync is not a function
|
fixes: #1114 |
|
@bmeck , @indexzero , @jcrugzz could u take a look too? |
|
I'll review today, please let me catch my breath a bit :) |
Sorrrry 😅 |
|
@akatona84 Would it be possible to also add test for the main fix, the one related to the configStore error? |
| @@ -4,7 +4,9 @@ var Configstore = require('configstore'); | |||
|
|
|||
| function initConfigFile(foreverRoot) { | |||
| try { | |||
| return new Configstore(JSON.parse(fs.readFileSync(path.join(foreverRoot, 'config.json')))); | |||
| return new Configstore('config', undefined, { | |||
There was a problem hiding this comment.
What's the idea behind this change?
There was a problem hiding this comment.
before this nconf was used, like this:
function initConfigFile(foreverRoot) {
return new nconf.File({file: path.join(foreverRoot, 'config.json')});
}
whatever changes you made in the configuration, it was saved eventually in this config.json file.
The first param should be an id and then the defaults and then options. It passed the parsed json to the first param before, which was used for creating the config file appending a ".json" to it so it became '[object Object].json' in ~/.config/configstore dir.
No backward compatibility, default configs was saved to this oddly named json and config.json in .forever dir was completely ignored.
I made it backward compatible and to use the original config file location.
| @@ -554,7 +554,6 @@ app.cmd('columns add :name', cli.addColumn = function (name) { | |||
| columns.push(name); | |||
|
|
|||
| forever.config.set('columns', columns); | |||
| forever.config.saveSync(); | |||
There was a problem hiding this comment.
Why are all of save calls removed without any replacement?
There was a problem hiding this comment.
Configstore persists the added configs right away, it doesn't require explicit save method call to save it to the file.
| var keys = Object.keys(forever.config.all), | ||
| conf = cliff.inspect(forever.config.all); |
There was a problem hiding this comment.
Configstore doesn't have store attribute, all() returns the whole
| describe('columns', function () { | ||
| this.timeout(5000); | ||
| it('manages columns successfully', function () { | ||
| return execCommand(`node bin/forever columns reset`, |
There was a problem hiding this comment.
this is testing whether forever columns reset works, it was throwing TypeError: forever.config.saveSync is not a function originally
| return execCommand(`node bin/forever columns rm uptime`, | ||
| { | ||
| expectedOutput: ['Removing column:', 'uptime'] | ||
| }); | ||
| }) | ||
| .then(function () { | ||
| return execCommand(`node bin/forever columns add uptime`, | ||
| { | ||
| expectedOutput: ['Adding column:', 'uptime'] | ||
| }); |
There was a problem hiding this comment.
these two just testing whether these commands are working too, and the config is actually persisted
| }); | ||
| }) | ||
| .then(function () { | ||
| return execCommand(`node bin/forever columns add uptime`, |
There was a problem hiding this comment.
also checking config persistence and a bit of functionality
The tests I added are using the Configstore and previously those commands are failing with 4.0.0. And thanks for checking! I appreciate it a lot :) |
|
@akatona84 Thanks a lot for contributing the fix! |
|
@kibertoad, Thank you for your help and your quick actions! 🍻 |
Fixes: TypeError: forever.config.saveSync is not a function