Skip to content

/ servo

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

Use the API base URL in Fetch APIs. #14632

Merged
merged 3 commits into from Dec 23, 2016
Merged

Use the API base URL in Fetch APIs. #14632

merged 3 commits into from Dec 23, 2016

Conversation

@Ms2ger
Copy link
Contributor

Ms2ger commented Dec 19, 2016 •
edited by larsbergstrom

This change is Reviewable
Ms2ger added 3 commits Dec 19, 2016
@Ms2ger
Make the multi-global tests more robust.
26cf0a7
@Ms2ger
Ensure that the API base URL is used in Fetch APIs.
fbed4d2
@Ms2ger
Use the API base URL in Fetch APIs.
Loading status checks…
75be030
@highfive
Copy link

highfive commented Dec 19, 2016

Heads up! This PR modifies the following files:

  • @fitzgen: components/script/dom/request.rs, components/script/dom/response.rs
  • @KiChjang: components/script/dom/request.rs, components/script/dom/response.rs
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Dec 20, 2016

r? @jdm
@highfive highfive assigned jdm and unassigned mbrubeck Dec 20, 2016
@jdm
Copy link
Member

jdm commented Dec 21, 2016

I am super wary of changing the multi-global tests without getting input from Domenic or others involved in their creation. I would prefer them to be submitted as a PR upstream instead.
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Dec 22, 2016

@domenic: could you have a look at the first two commits here? The first avoids frames[i] which is not yet implemented in Servo, the second delays accessing contentWindow (not sure what's up there exactly); neither should affect the meaning of the test.
@domenic
Copy link

domenic commented Dec 22, 2016

LGTM; the second commit that ensures it's testing the API base URL is a nice one.
@jdm
Copy link
Member

jdm commented Dec 22, 2016

Thanks!
@bors-servo: r+
@bors-servo
Copy link
Contributor

bors-servo commented Dec 22, 2016

📌 Commit 75be030 has been approved by jdm
@bors-servo
Copy link
Contributor

bors-servo commented Dec 22, 2016

⌛ Testing commit 75be030 with merge d5c007f...
bors-servo added a commit that referenced this pull request Dec 22, 2016
@bors-servo
Auto merge of #14632 - servo:multi-global-fetch, r=jdm
Loading status checks…
d5c007f 
Use the API base URL in Fetch APIs.

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14632)
<!-- Reviewable:end -->
@larsbergstrom
Copy link
Contributor

larsbergstrom commented Dec 23, 2016

@bors-servo force clean retry
@bors-servo
Copy link
Contributor

bors-servo commented Dec 23, 2016

⌛ Testing commit 75be030 with merge 89cdcbc...
bors-servo added a commit that referenced this pull request Dec 23, 2016
@bors-servo
Auto merge of #14632 - servo:multi-global-fetch, r=jdm
Loading status checks…
89cdcbc 
Use the API base URL in Fetch APIs.

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14632)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Dec 23, 2016

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-gnu-dev
@bors-servo bors-servo merged commit 75be030 into master Dec 23, 2016
4 checks passed
4 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
dependency-ci Dependencies checked
Details
homu Test successful
Details
@Ms2ger Ms2ger deleted the multi-global-fetch branch Dec 23, 2016
@KiChjang KiChjang mentioned this pull request Dec 23, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jdm jdm

Labels
S-blocked-on-external
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
@Ms2ger @highfive @jdm @domenic @bors-servo @larsbergstrom @mbrubeck
You can’t perform that action at this time.