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

core(config): split out config helpers #9003

Merged
merged 4 commits into from
May 25, 2019
Merged

Conversation

patrickhulce
Copy link
Collaborator

Summary
Smallest portion I can see to split out from #8374. I made separate copy and then modify commits to make it easier to review, but it didn't really help much since config is so massive. Sorry! Part 1 of 4.

  1. (This PR) Split apart some config logic into a helper file.
  2. Add a dedicated ID to audit and gatherer definitions so that they are more easily referenceable. This also has the happy side effect of enabling the use of options in plugin audits in step 3 :D
  3. Enforce that every plugin audit id is prefixed with <plugin-id>. It will still be able to reference our existing core audit implementations.
  4. Load each audit when loading a plugin to check its requiredArtifacts against a set of blessed artifacts.

Besides blessed artifacts and validating plugin audits at load time, the primary benefit of this effort is that plugins can start to reuse core audits with their own options. Immediate use cases I'd like to offer are a simulated 3G performance plugin and simulated warm load performance plugin. i.e. user gets all three scores with just 1 LH run.

Related Issues/PRs
#6747 #8374

Copy link
Member

@exterkamp exterkamp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a pretty simple extraction of code into a new file, LGTM, just some nits.

lighthouse-core/config/config-helpers.js Outdated Show resolved Hide resolved
lighthouse-core/config/config-helpers.js Show resolved Hide resolved
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