Create a speccy.yml Config File for common options #93
Conversation
|
This PR only seems to cover the I know there are a lot of options, but we talked about how each of them should be able to be set in the config file... and this PR doesn't do any of that. |
lint.js
Outdated
| @@ -80,10 +85,14 @@ More information: https://speccy.io/rules/#${rule.name} | |||
|
|
|||
| const command = async (file, cmd) => { | |||
| const verbose = cmd.quiet ? 1 : cmd.verbose; | |||
| let rules = [].concat(cmd.rules); | |||
There was a problem hiding this comment.
No need to concat a single array with itself. This is just making a copy as there is only one array there.
lint.js
Outdated
|
|
||
| linter.initialize(); | ||
|
|
||
| await loader.loadRuleFiles(cmd.rules, { verbose }); | ||
| if(fs.existsSync(process.env["NODE_CONFIG_DIR"] + "/default.yml")) { |
There was a problem hiding this comment.
This is what you were doing before, and we moved away from it. You dont need to check for the existance of a config file, just let rules = [].concat(cmd.rules, config.get('lint.rules')); and if there is no config file there will be no rules!
There was a problem hiding this comment.
I overlooked the config.has() method and incorrectly compensated for the resulting errors. That has been corrected.
test/linter.test.js
Outdated
| const rules = config.get('lint.rules'); | ||
|
|
||
| it('gets rules', () => { | ||
| if(rules); |
There was a problem hiding this comment.
This test is not doing anything?
dangoosby
force-pushed
the
feature/speccy-config
branch
from
ecf3ad1
to
cb5d188
Compare
| @@ -40,6 +46,12 @@ const command = async (specFile, cmd) => { | |||
| verbose | |||
| }); | |||
|
|
|||
| let port = cmd.port; | |||
There was a problem hiding this comment.
The cli interface should probably override given config file values, not the other way around.
This does make things a bit tricky trying to do it in the command files, because cmd.port has a default value, set in speccy.js
This means that instead of just doing something simple like this:
const port = config.get('serve.port') || cmd.port;
... we'd need to use config.get() in speccy.js and use it to define the default values.
That might look like this in speccy.js:
.option('-p, --port [value]', 'port on which the server will listen', config.get('serve.port') || 5000)
This means the CLI option is used if provided, then serve.port is inspected. If no value is set, it'll use 5000 as the default port!
Similar stuff will have to be done for the other options too.
dangoosby
force-pushed
the
feature/speccy-config
branch
from
eea265b
to
3ee560d
Compare
dangoosby
force-pushed
the
feature/speccy-config
branch
from
3ee560d
to
a3f7b5e
Compare
philsturgeon
changed the title
|
Closing in favour of #102. Thanks for the prototype and hard work on this! |
No description provided.