Closed brendankenny closed 6 years ago
for rawValue -> expectedValue
, I think @paulirish mentioned these are for figuring out the value type for the report. Maybe it should be rawValue -> valueIsOfType
? IDK
In my opinion rawValue is a good choice. The value is raw => whatever comes out of the audit. In config i'm ok with expectedValue.
What might also help is maybe a config generator which we once discussed when @paulirish had to leave. Where more information can be given as well for each property
categorizable => maybe should be showInReport ?
In my opinion rawValue is a good choice. The value is raw => whatever comes out of the audit. In config i'm ok with expectedValue.
Yes, this is for the config file only, where expectedValue
is much clearer to someone writing a custom config. Agreed that rawValue
on the audit result itself is still a good choice.
done enough
The config file was put together very quickly and has evolved somewhat haphazardly, so it's time to take a look at cleaning it up. Here's a list of properties in the config file I've witnessed confuse some users, and a suggestion for how they could be changed. Feel free to bikeshed away and add more. After this we can start documenting re: #672
Note that if we change these, anyone with a custom config will be broken, so we probably want to do this right once and then stick to them until LH2.0.
[ ]
passes
->gathererPasses
Example. "What is a pass?" We need to make clear this is for the gathering stage, but no need to nest within another object. @paulirish also had some notion around a name that makes it clearer these are each a page reload.
[x]
network
->recordNetwork
Example. Needs to be clear that this isn't offline/online, but about getting a record of the network requests at the end of the pass. If not present, defaults to
false
.[x]
trace
->recordTrace
Example. Less necessary, but useful to be parallel to
recordNetwork
. Defaults tofalse
.[x]
loadPage
-> 💥Example. I'd still argue that
loadPage
isn't necessary, and the extra option is more overhead for devs using LH and for us testing LH. We haven't run into an audit that needs it or has used it yet. Moreover, it's actually non-functional right now, and fixing it would take as much effort as implementing it in the future if someone ever actually needed it. Removing it would involve deleting a single conditional and then from the various config files we have lying around. Currently defaults totrue
if not present, which violates assumptions about truthy/falsy.Under
aggregations
:[ ]
categorizable
-> ?Example. It would be nice to have a more descriptive name. This is only(?) for report generation, but not at all clear from the name what it does or if it's necessary when writing a custom aggregation. It's possible documenting it may be sufficient.
[x]
criteria
->audits
?Example. Not completely sure what we should use here, but when writing his own config, @ebidel wondered if these are just audits, why not just call them that. Would make it clear that the keys inside refer to audit names.
[ ]
items
-> ?Example. Something more descriptive. Current choice might as well be
things
orarrayValues
:)[x]
rawValue
->expectedValue
Example. From #677. I think the intention was to make the name parallel to
auditResult.rawValue
, butrawValue
doesn't mean anything to you inside the config file.Any other ones?