core(fr): add audit mode filter #12649
Conversation
| * @return {LH.Config.FRConfig['audits']} | ||
| */ | ||
| function filterAuditsByAvailableArtifacts(audits, availableArtifacts) { | ||
| function filterAudits(audits, availableArtifacts, mode) { |
There was a problem hiding this comment.
separate function filterAuditsByGatherMode? the BySomeVerboseSingleThing was intentional for clarity :)
My lessons from the way config evolved the past 5 years and how important these files are was to be as excessively clear as possible in v2 ;)
There was a problem hiding this comment.
I tried filterAuditsByGatherModeAndAvailableArtifacts but that was too long. I'm ok excluding one of the filters from the name if you are though.
There was a problem hiding this comment.
Now that I think about it filterAuditsByAvailableArtifacts makes more sense because we will filtering by artifacts most of the time.
There was a problem hiding this comment.
WDYT about two functions?
filterAuditsByAvailableArtifacts
filterAuditsByGatherMode
filterCategories invokes both of them in sequence
types/audit.d.ts
Outdated
| @@ -57,6 +57,8 @@ declare global { | |||
| __internalOptionalArtifacts?: Array<keyof Artifacts>; | |||
| /** A string identifying how the score should be interpreted for display. */ | |||
| scoreDisplayMode?: Audit.ScoreDisplayMode; | |||
| /** A list of gather modes that this audit is applicable to. */ | |||
| applicableModes?: Gatherer.GatherMode[], | |||
There was a problem hiding this comment.
WDYT about matching supportedModes? was the use of applicable instead an intentional differentiator?
There was a problem hiding this comment.
It was different intentionally to avoid mismatch with gatherers. supportedModes does make sense though so I'm fine going back.
Feels like a solid v9 breaking change to make :) |
Co-authored-by: Patrick Hulce <[email protected]>
Implementation of our ideas in #12595 (comment)
This feels good to me, but I would be receptive to making
applicableModesrequired for two reasons: