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): enable unused-javascript by default #9854

Merged
merged 5 commits into from
Nov 6, 2019

Conversation

patrickhulce
Copy link
Collaborator

Summary
See the excellent summary in #9853 :)

Related Issues/PRs
closes #9853

@@ -16,7 +16,7 @@ class JsUsage extends Gatherer {
*/
async beforePass(passContext) {
await passContext.driver.sendCommand('Profiler.enable');
await passContext.driver.sendCommand('Profiler.startPreciseCoverage');
await passContext.driver.sendCommand('Profiler.startPreciseCoverage', {detailed: false});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To be certain we're on the same page, is this what was meant by function-level @bmeurer? Or is it only getBestEffortCoverage that has the characteristics we're discussing?

Copy link
Member

Choose a reason for hiding this comment

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

Yes this gives you function-level coverage. The best effort coverage is not supported since it's unreliable and misleading, and we will likely remove that in the future.

@brendankenny
Copy link
Member

see if we can also cc @sigurdschneider in here :)

@connorjclark
Copy link
Collaborator

Do we want this for 6.0?

@patrickhulce
Copy link
Collaborator Author

Do we want this for 6.0?

I say we do it :) Any more objections?

Copy link
Member

@paulirish paulirish left a comment

Choose a reason for hiding this comment

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

wfm!

@@ -9,25 +9,6 @@
const fullConfig = {
Copy link
Member

Choose a reason for hiding this comment

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

delete the file maybe? breaking change opportunity and all that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that ended up being somewhat large of a change, so new PR #9930 👍

@paulirish paulirish merged commit 713a70a into master Nov 6, 2019
@brendankenny brendankenny deleted the enable_js_coverage branch November 7, 2019 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Explore re-enabling unused JS audit by default
6 participants