Skip to content

core: remove the redirect pass and redirects-http audit#12643

Merged
paulirish merged 7 commits intomasterfrom
rmredirects
Oct 20, 2021
Merged

core: remove the redirect pass and redirects-http audit#12643
paulirish merged 7 commits intomasterfrom
rmredirects

Conversation

@paulirish
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
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
Copy Markdown
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
Copy Markdown
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