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 the redirect pass and redirects-http audit #12643

Merged
merged 7 commits into from
Oct 20, 2021
Merged

Conversation

paulirish
Copy link
Member

After consideration, we don't think this audit serves much value anymore.

If one is building a PWA, it clearly needs to be on HTTPS. I mean technically they could have a functional HTTP site, but in 2021, you get pretty shamed in browser UI if you do that. On top of that, Chrome's change to default HTTPS navigation mitigates the MITM concern that's been a primary motivation.

We've spoked with the installability folks and they're chill with it.

part of #11822

_

Breaking, so it'll have to wait to land, sadly, but I wanted a PR to track it early.

@patrickhulce
Copy link
Collaborator

To fix #12636 sooner, WDYT about landing a version of this PR that removes the artifact and the audit always gets marked notApplicable but we keep the result object around for key access backcompat?

@paulirish
Copy link
Member Author

To fix #12636 sooner, WDYT about landing a version of this PR that removes the artifact and the audit always gets marked notApplicable but we keep the result object around for key access backcompat?

Good idea.

@paulirish paulirish mentioned this pull request Sep 14, 2021
15 tasks
@paulirish paulirish marked this pull request as ready for review October 20, 2021 21:04
@paulirish paulirish requested a review from a team as a code owner October 20, 2021 21:04
@paulirish paulirish requested review from connorjclark and removed request for a team October 20, 2021 21:04
@@ -140,7 +140,7 @@ function resolveArtifactsToDefns(artifacts, configDir) {

const gatherer = resolveGathererToDefn(gathererJson, coreGathererList, configDir);
if (!isFRGathererDefn(gatherer)) {
throw new Error(`${gatherer.instance.name} gatherer does not support Fraggle Rock`);
throw new Error(`${gatherer.instance.name} gatherer does not have a Fraggle Rock meta obj`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

idk about this change specifically, but it certainly is hidden in this huge unrelated PR :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I was chatting with Adam about it. the config-test item that also changed had me looking at this.

adam suggested adding a todo to adjust the error message, but instead i decided to just do it. agree it's fairly distinct from the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants