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

Change Response's statusText default value from 'Ok' to an empty string #22244

Merged
merged 1 commit into from Nov 28, 2018

Conversation

@georgeroman
Copy link
Contributor

georgeroman commented Nov 21, 2018

Closes #22238


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • There are tests for these changes

This change is Reviewable

@highfive
Copy link

highfive commented Nov 21, 2018

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @nox (or someone else) soon.

@highfive
Copy link

highfive commented Nov 21, 2018

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/webidls/Response.webidl
  • @KiChjang: components/script/dom/webidls/Response.webidl
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Nov 21, 2018

Error syncing changes upstream. Logs saved in error-snapshot-1542840208228.

@jdm
Copy link
Member

jdm commented Nov 21, 2018

Oh, looks like #22235 means that we have not pulled the most recent changes from the upstream tests yet. That's frustrating.

@nox nox assigned jdm and unassigned nox Nov 22, 2018
@bors-servo
Copy link
Contributor

bors-servo commented Nov 24, 2018

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

@jdm
Copy link
Member

jdm commented Nov 24, 2018

If you rebase, the test should show different results now.

@nox
Copy link
Member

nox commented Nov 27, 2018

@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented Nov 27, 2018

Trying commit 8133f31 with merge 29d627a...

bors-servo added a commit that referenced this pull request Nov 27, 2018
…, r=<try>

Change Response's statusText default value from 'Ok' to an empty string

<!-- Please describe your changes on the following line: -->
Closes #22238

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors

<!-- Either: -->
- [X] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/22244)
<!-- Reviewable:end -->
@nox
Copy link
Member

nox commented Nov 27, 2018

Pretty sure this needs expectation changes.

@bors-servo
Copy link
Contributor

bors-servo commented Nov 27, 2018

💔 Test failed - linux-rel-wpt

@nox
Copy link
Member

nox commented Nov 27, 2018

{"status": "PASS", "group": "default", "message": null, "stack": null, "subtest": "Check Response's clone with default values, without body", "test": "/fetch/api/response/response-clone.html", "line": 71008, "action": "test_result", "expected": "FAIL"}
{"status": "PASS", "group": "default", "message": null, "stack": null, "subtest": "Check default value for statusText attribute", "test": "/fetch/api/response/response-init-001.html", "line": 75363, "action": "test_result", "expected": "FAIL"}
@CYBAI
Copy link
Collaborator

CYBAI commented Nov 27, 2018

@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented Nov 27, 2018

Trying commit 8b43efc with merge 0300272...

bors-servo added a commit that referenced this pull request Nov 27, 2018
…, r=<try>

Change Response's statusText default value from 'Ok' to an empty string

<!-- Please describe your changes on the following line: -->
Closes #22238

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors

<!-- Either: -->
- [X] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/22244)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 27, 2018

💔 Test failed - linux-rel-wpt

@jdm
Copy link
Member

jdm commented Nov 27, 2018

The code that deals with blob responses in scheme_fetch will need to provide an OK value per https://fetch.spec.whatwg.org/#scheme-fetch .

@jdm
Copy link
Member

jdm commented Nov 28, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Nov 28, 2018

📌 Commit 73f11d6 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Nov 28, 2018

Testing commit 73f11d6 with merge fdfe0a1...

bors-servo added a commit that referenced this pull request Nov 28, 2018
…, r=jdm

Change Response's statusText default value from 'Ok' to an empty string

<!-- Please describe your changes on the following line: -->
Closes #22238

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors

<!-- Either: -->
- [X] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/22244)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 28, 2018

💔 Test failed - linux-rel-css

@jdm
Copy link
Member

jdm commented Nov 28, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Nov 28, 2018

Testing commit 73f11d6 with merge 06f453a...

bors-servo added a commit that referenced this pull request Nov 28, 2018
…, r=jdm

Change Response's statusText default value from 'Ok' to an empty string

<!-- Please describe your changes on the following line: -->
Closes #22238

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors

<!-- Either: -->
- [X] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/22244)
<!-- Reviewable:end -->
@bors-servo bors-servo merged commit 73f11d6 into servo:master Nov 28, 2018
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@jdm jdm mentioned this pull request Oct 7, 2019
4 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.