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.

Sign up for GitHub

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

Jump to bottom

Redirects to non-HTTP(S) schemes are a network error #3095

Merged
merged 3 commits into from May 30, 2016
Merged

Redirects to non-HTTP(S) schemes are a network error #3095

merged 3 commits into from May 30, 2016

Conversation

annevk
Copy link
Member

@annevk annevk commented May 30, 2016

See whatwg/fetch#309 for context.

@wpt-pr-bot
Copy link
Collaborator

Reviewers for this pull request are: @jdm and @youennf.

@hoppipolla-critic-bot
Copy link

Critic review: https://critic.hoppipolla.co.uk/r/6571

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

jdm
jdm reviewed May 30, 2016
View reviewed changes
fetch/api/redirect/redirect-schemes.html
]
;tests.forEach(function(f) {
promise_test(function(t) {
return promise_rejects(t, new TypeError(), f)
Copy link
Contributor

Choose a reason for hiding this comment

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

The indentation in this file is super funky. Please add a newline after the ;, as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, removed the tabs. Leading with ";" where needed is sort of my preferred style, but I guess I can change it if you want.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can't say I've ever come across it before, and it feels like it sticks out among the prevailing style in the rest of the tests.

@annevk annevk merged commit d3451d2 into master May 30, 2016
@annevk annevk deleted the annevk-fetch-redirect-scheme branch May 31, 2016 08:52
@annevk annevk mentioned this pull request Sep 30, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
fetch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@annevk @wpt-pr-bot @hoppipolla-critic-bot @jdm