Skip to content
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

Config: prevent meaningless empty arrays override options in file #278

Merged
merged 2 commits into from
Jun 16, 2019

Conversation

mdaloia
Copy link
Contributor

@mdaloia mdaloia commented Jan 30, 2019

It fixes the impossibility that the rules and skip options specified in speccy.yaml config file are used when these options are not override from command line.

The Commander.js configuration has an empty array as a default/initial value for rules and skip. If you execute the lint command from the command line without specifying those arguments, when the config is loaded those 'supplied' literal config values (empty arrays) overrides the ones defined in a speccy.yaml file (because they are not undefined).

There is a test validating the config hierarchy but, as the configuration passed to init() is handcrafted, it wasn't simulating the output of Commander.js read options.
Now, a test simulating this case was added.

@pderaaij
Copy link
Contributor

Looks good to me, @djtarazona this one can be tagged along in a new release.

@djtarazona
Copy link
Contributor

@mdaloia Feel free to resolve the conflicts and I'll take another look at merging this in. Thanks!

The tests in lint.test.js fails if you have a speccy.yaml config file
with 'verbose: true' because all the tests spying on the messages
sent to the console through log(), warn() and error() methods and
asserting the quantity of calls.

Making it explicit as the default config for those tests allows us to
run them even if we have a speccy.yaml in the root directory.
@mdaloia mdaloia force-pushed the fix-empty-arrays-in-default-config branch from e9ca21d to cc775db Compare May 19, 2019 17:55
It fixes the impossibility that the 'rules' and 'skip' options specified
in speccy.yaml config file are used when these options are not
override from command line.

The Commander.js configuration has an empty array as a default/initial
value for 'rules' and 'skip'. If you execute the lint command from the
command line without specifying those arguments, when the config is
loaded those 'supplied' literal config values (empty arrays) overrides
the ones defined in a speccy.yaml file (because they are not 'undefined').

There is a test validating the config hierarchy but, as the configuration
passed to init() is handcrafted, it wasn't simulating the output of
Commander.js read options.
Now, a test simulating this case was added.
@mdaloia mdaloia force-pushed the fix-empty-arrays-in-default-config branch from cc775db to a62c060 Compare May 19, 2019 18:06
@mdaloia
Copy link
Contributor Author

mdaloia commented May 19, 2019

@djtarazona Done! Fixed and rebased with master. Thanks

@djtarazona djtarazona merged commit 36c6b4c into wework:master Jun 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants