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: remove no-websql audit #6293

Merged
merged 4 commits into from
Oct 17, 2018
Merged

core: remove no-websql audit #6293

merged 4 commits into from
Oct 17, 2018

Conversation

patrickhulce
Copy link
Collaborator

@patrickhulce patrickhulce commented Oct 16, 2018

Summary
This audit is relevant to only 0.6% of sites on HTTPArchive (only 8k out of ~1.3M) . The websql gatherer takes at least 500ms for every site that passes the audit, so we're spending 1,200,000 seconds a month just on WPT doing nothing :)

My claim is that this audit would not pass our new-audit criteria doc if we were to validate it today and should be removed.

Related Issues
thought of while investigating #6272

See Query
SELECT
  JSON_EXTRACT_SCALAR(report, '$.audits.no-websql.score') as score, COUNT(*)
FROM
  httparchive.lighthouse.2018_09_01_mobile
 GROUP BY score

@brendankenny
Copy link
Member

screen shot 2018-10-16 at 10 18 43

someone playing around this this?

@brendankenny
Copy link
Member

oh, maybe it's new actions or whatever

Copy link
Member

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

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

Yeah! Definitely feels like the kind of thing that should come through Chrome's deprecations (which should be free...) instead of a slow check by us.

Need to also delete WebSQL in artifacts.d.ts

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.

LGTM

@patrickhulce
Copy link
Collaborator Author

Need to also delete WebSQL in artifacts.d.ts

@brendankenny do you have a way for force vscode show the .d.ts files and have them in find/replace

        "**/*.d.ts": {
            "when": "$(basename).ts"
        }

hasn't been doing the trick clearly :)

@brendankenny
Copy link
Member

oh, no, and I hadn't even noticed that was a problem...

I was thinking that maybe we need to reverse artifacts.d.ts and have it pull the artifact types off of the gatherers that produce them (which would have flagged the change in this case), but I believe our earlier reasoning was that by having the source of truth in that file, bugs in gatherers would be more easily caught, which still seems sound...

@patrickhulce patrickhulce merged commit bf069a6 into master Oct 17, 2018
@patrickhulce patrickhulce deleted the remove_websql branch October 17, 2018 00:44
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.

4 participants