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

Jump to bottom

Partially revert "Add another https port (#24016)" #24394

Merged
merged 2 commits into from Jun 30, 2020
Merged

Partially revert "Add another https port (#24016)" #24394

merged 2 commits into from Jun 30, 2020

Conversation

stephenmcgruer
Copy link
Contributor

@stephenmcgruer stephenmcgruer commented Jun 30, 2020 •
edited

This reverts the change to common/get-host-info.sub.js in commit
95cdf1f.

Adding another HTTPS port broke there broke the Chromium WPT
Importer (as the pinned tooling failed to sub common/get-host-info.sub.js
due to indexing out of bounds).

@stephenmcgruer
Revert "Add another https port (#24016)"
b95c4a2 
This reverts commit 95cdf1f.

Adding another HTTPS port broke both wpt.live (as its config no longer
matched the number of ports asserted in wptserve) and the Chromium WPT
Importer (as our pinned tooling failed to sub
common/get-host-info.sub.js due to indexing out of bounds).
@wpt-pr-bot wpt-pr-bot added assets infra serve wptrunner The automated test runner, commonly called through ./wpt run labels Jun 30, 2020
@stephenmcgruer stephenmcgruer mentioned this pull request Jun 30, 2020
Closed
@wpt-pr-bot wpt-pr-bot requested a deployment to wpt-preview-24394 June 30, 2020 12:43 Pending
Hexcles
Hexcles approved these changes Jun 30, 2020
View reviewed changes
Copy link
Member

@Hexcles Hexcles left a comment

Choose a reason for hiding this comment

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

LGTM

@stephenmcgruer stephenmcgruer changed the title Revert "Add another https port (#24016)" Partially revert "Add another https port (#24016)" Jun 30, 2020
@wpt-pr-bot wpt-pr-bot had a problem deploying to wpt-preview-24394 June 30, 2020 13:37 Error
Hexcles
Hexcles approved these changes Jun 30, 2020
View reviewed changes
Copy link
Member

@Hexcles Hexcles left a comment

Choose a reason for hiding this comment

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

LGTM

Tangentially, maybe we should make index-out-of-bound non-fatal in templates? But I don't have a great idea how to do that without pushing the error further down the line in a more subtle way.

@stephenmcgruer stephenmcgruer merged commit c9d349e into master Jun 30, 2020
@stephenmcgruer stephenmcgruer deleted the smcgruer/revert-https branch June 30, 2020 16:01
@stephenmcgruer stephenmcgruer mentioned this pull request Jun 30, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hexcles Hexcles Hexcles approved these changes

@deniak deniak Awaiting requested review from deniak

@jgraham jgraham Awaiting requested review from jgraham

@LukeZielinski LukeZielinski Awaiting requested review from LukeZielinski

@zqzhang zqzhang Awaiting requested review from zqzhang

Assignees

@jgraham jgraham

Labels
assets infra serve wptrunner The automated test runner, commonly called through ./wpt run
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@stephenmcgruer @Hexcles @jgraham @wpt-pr-bot