-
Notifications
You must be signed in to change notification settings - Fork 63
-
Notifications
You must be signed in to change notification settings - Fork 63
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
Default configuration section in Readme has wrong key name #91
Comments
Why merging a PR which is wrong? The tests using |
To make it clear, the PR #89 only changed the documentation in the Readme, not the actual code, so I think the description here is a bit misleading. |
Hi @einSelbst, yes, ok, #89 only changes documentation in the README (which is the only one documentation available now), but people that want to use For instance, that was exactely my case: i used |
FWIW a made the PR in the best intention, especially to prevent the situation you experienced, which I, kind of, experienced before as I mentioned regarding #85 . I'm totally fine if it is reverted and in fact just commented to explain my original intention to make it even easier to do so. |
Fixes wrong key for defaults object pa11y#91
Fixes wrong key for defaults object #91
A recent PR (#89) changed the json key of the default configuration option from
defaults
todefault
.As far as I can tell this is incorrect? Config options are not picked up if specified under the
default
key. Using the previous value ofdefaults
works correctly however.For example, according to the current documentation this should set the timeout to 1ms (and therefore immediately timeout as an easily reproducible test case). But the config is not picked up, and the request works fine
Using the previous key of
defaults
works fine however, and the short timeout is correctly picked up here (and so the request immediately timeout).See https://github.com/pa11y/pa11y-ci/blob/master/bin/pa11y-ci.js#L192 which is referencing
config.defaults
.The integration tests also still reference
defaults
. See https://github.com/pa11y/pa11y-ci/blob/master/test/integration/mock/config/defaults.jsonTested using pally-cli 2.3.0
Could the documentation change please be reverted? Or the code updated to look for the
default
key.Thanks for a very useful tool!
The text was updated successfully, but these errors were encountered: