Use hosts-replaced URL only when loading resources by jgraham · Pull Request #6416 · servo/servo

jgraham · 2015-06-18T13:16:46Z

highfive · 2015-06-18T13:16:50Z

Warning

These commits modify unsafe code. Please review it carefully!

hoppipolla-critic-bot · 2015-06-18T13:16:52Z

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

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.

jgraham · 2015-06-18T13:18:56Z

r? @Manishearth I'm not really sure that this is perfect; I couldn't decide which urls to put in error messages, and it generally feels a bit fragile.

jdm · 2015-06-19T16:30:16Z

components/script/Cargo.toml

Nope! net_traits exists for a reason.

Oh look, a @jdm!

You want the whole of the HOSTS_FILE stuff moved into net_traits then?

Yes. Or make it polymorphic on a trait in net_traits. The former is okay.

Manishearth · 2015-06-19T22:16:01Z

Mostly LGTM except for the dependency.

Also, there should be some comments about what the various _url variables mean.

Ms2ger · 2015-06-20T12:20:44Z

components/compositing/constellation.rs:602: (much) overlong line
components/compositing/constellation.rs:605: (much) overlong line

waddlesplash · 2015-06-23T18:59:47Z

components/net_traits/Cargo.toml

Any reason you got rid of the EOL at the end of the file?

bors-servo · 2015-06-25T20:06:01Z

☔ The latest upstream changes (presumably #6460) made this pull request unmergeable. Please resolve the merge conflicts.

jdm · 2015-07-06T14:26:25Z

@bors-servo: r+

bors-servo · 2015-07-06T14:26:26Z

📌 Commit 15ca46f has been approved by jdm

bors-servo · 2015-07-06T14:52:01Z

⌛ Testing commit 15ca46f with merge 00116d6...

Use hosts-replaced URL only when loading resources  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6416)

bors-servo · 2015-07-06T15:16:44Z

💔 Test failed - gonk

jdm · 2015-08-05T00:41:23Z

@bors-servo: r+
Makes sense.

bors-servo · 2015-08-05T00:41:23Z

📌 Commit 49a1cfe has been approved by jdm

bors-servo · 2015-08-05T01:43:14Z

⌛ Testing commit 49a1cfe with merge aaa98d1...

Use hosts-replaced URL only when loading resources  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6416)

bors-servo · 2015-08-05T02:21:35Z

💔 Test failed - mac1

jdm · 2015-08-05T02:43:08Z



/XMLHttpRequest/timeout-cors-async.htm
--------------------------------------
FAIL XMLHttpRequest: timeout event and cross-origin request

jdm · 2015-08-05T02:43:22Z

This PR really doesn't want to merge!

jgraham · 2015-08-05T09:44:11Z

Seems like there shouldn't have been any metadata updates at all. Which makes some sense as wptrunner hasn't been updated to use the right host name.

jdm · 2015-08-05T13:29:14Z

@bors-servo: r+

bors-servo · 2015-08-05T13:29:15Z

📌 Commit f52276d has been approved by jdm

bors-servo · 2015-08-05T14:01:05Z

⌛ Testing commit f52276d with merge 8602d01...

Use hosts-replaced URL only when loading resources  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6416)

bors-servo · 2015-08-05T15:21:09Z

☀️ Test successful - android, gonk, linux1, linux2, linux3, mac1, mac2, mac3

Update to unicode-bidi 0.2.1 Also includes some missing changes from #6416.  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6998)

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jun 18, 2015

jdm reviewed Jun 19, 2015
View reviewed changes

Manishearth added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jun 19, 2015

Ms2ger assigned Manishearth Jun 20, 2015

jgraham force-pushed the hosts_replaced branch from b3b392d to 159f4b6 Compare June 22, 2015 19:35

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Jun 22, 2015

jgraham force-pushed the hosts_replaced branch from 159f4b6 to 295cd53 Compare June 22, 2015 22:46

waddlesplash reviewed Jun 23, 2015
View reviewed changes

components/net_traits/Cargo.toml Outdated

Copy link
Copy Markdown

waddlesplash Jun 23, 2015

Choose a reason for hiding this comment

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

Any reason you got rid of the EOL at the end of the file?

jgraham force-pushed the hosts_replaced branch from 295cd53 to 617ef72 Compare June 23, 2015 21:44

jgraham force-pushed the hosts_replaced branch from 617ef72 to 9ded0ae Compare June 26, 2015 20:37

jdm added the S-needs-rebase There are merge conflict errors. label Jun 26, 2015

jgraham force-pushed the hosts_replaced branch 2 times, most recently from 8cd704c to 15ca46f Compare June 29, 2015 18:19

jgraham removed the S-needs-rebase There are merge conflict errors. label Jun 29, 2015

jdm added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jul 6, 2015

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Aug 4, 2015

jdm added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Aug 5, 2015

jdm added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Aug 5, 2015

Use hosts-replaced URL only when loading resources

f52276d

jgraham force-pushed the hosts_replaced branch from 49a1cfe to f52276d Compare August 5, 2015 09:43

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Aug 5, 2015

jdm added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Aug 5, 2015

bors-servo merged commit f52276d into servo:master Aug 5, 2015

This was referenced Aug 5, 2015

Move EventTargetTypeId/NodeTypeId to DOMClass #6615

Closed

compositing: Split Servo up into multiple sandboxed processes. #6884

Closed

mbrubeck mentioned this pull request Aug 5, 2015

Update to unicode-bidi 0.2.1 #6998

Merged

Uh oh!

Conversation

jgraham commented Jun 18, 2015

Uh oh!

highfive commented Jun 18, 2015

Uh oh!

hoppipolla-critic-bot commented Jun 18, 2015

Uh oh!

jgraham commented Jun 18, 2015

Uh oh!

jdm Jun 19, 2015

Choose a reason for hiding this comment

Uh oh!

jgraham Jun 19, 2015

Choose a reason for hiding this comment

Uh oh!

Manishearth Jun 19, 2015

Choose a reason for hiding this comment

Uh oh!

Manishearth commented Jun 19, 2015

Uh oh!

Ms2ger commented Jun 20, 2015

Uh oh!

waddlesplash Jun 23, 2015

Choose a reason for hiding this comment

Uh oh!

bors-servo commented Jun 25, 2015

Uh oh!

jdm commented Jul 6, 2015

Uh oh!

bors-servo commented Jul 6, 2015

Uh oh!

bors-servo commented Jul 6, 2015

Uh oh!

bors-servo commented Jul 6, 2015

Uh oh!

jdm commented Aug 5, 2015

Uh oh!

bors-servo commented Aug 5, 2015

Uh oh!

bors-servo commented Aug 5, 2015

Uh oh!

bors-servo commented Aug 5, 2015

Uh oh!

jdm commented Aug 5, 2015

Uh oh!

jdm commented Aug 5, 2015

Uh oh!

jgraham commented Aug 5, 2015

Uh oh!

jdm commented Aug 5, 2015

Uh oh!

bors-servo commented Aug 5, 2015

Uh oh!

bors-servo commented Aug 5, 2015

Uh oh!

bors-servo commented Aug 5, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants