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

Implement DOM APIs for ChannelSplitterNode #22648

Merged
merged 2 commits into from Jan 12, 2019

Conversation

Projects
None yet
6 participants
@collares
Copy link
Contributor

collares commented Jan 8, 2019

Based on #21591. Fixes #21558. I tried to update the expected results for WPT using "./mach update-wpt"; let me know if I got something wrong.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix the second half of #21558
  • There are web-platform-tests tests for these changes
  • These changes do not require tests because ___

This change is Reviewable

@highfive

This comment has been minimized.

Copy link

highfive commented Jan 8, 2019

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

@highfive

This comment has been minimized.

Copy link

highfive commented Jan 8, 2019

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/webidls/BaseAudioContext.webidl, components/script/dom/mod.rs, components/script/dom/webidls/AudioNode.webidl, components/script/dom/audionode.rs, components/script/dom/baseaudiocontext.rs and 2 more
  • @KiChjang: components/script/dom/webidls/BaseAudioContext.webidl, components/script/dom/mod.rs, components/script/dom/webidls/AudioNode.webidl, components/script/dom/audionode.rs, components/script/dom/baseaudiocontext.rs and 2 more
Show resolved Hide resolved components/script/dom/audionode.rs Outdated
Show resolved Hide resolved components/script/dom/audionode.rs Outdated
Show resolved Hide resolved components/script/dom/audionode.rs Outdated
Show resolved Hide resolved components/script/dom/channelsplitternode.rs Outdated
@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Jan 9, 2019

overall looks good! minor fixes (there was a bug in the merger code which may have misled you here, my bad)

@collares collares force-pushed the collares:ChannelSplitterNode branch from 4b61f1e to 7f97a55 Jan 9, 2019

@collares collares force-pushed the collares:ChannelSplitterNode branch from 7f97a55 to ab01190 Jan 9, 2019

@collares

This comment has been minimized.

Copy link
Contributor Author

collares commented Jan 9, 2019

Manish and Keith, I appreciate the review comments :) Thanks! Updated the patch.

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Jan 9, 2019

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 9, 2019

📌 Commit ab01190 has been approved by Manishearth

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 10, 2019

⌛️ Testing commit ab01190 with merge d1d24a0...

bors-servo added a commit that referenced this pull request Jan 10, 2019

Auto merge of #22648 - collares:ChannelSplitterNode, r=Manishearth
Implement DOM APIs for ChannelSplitterNode

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

Based on #21591. Fixes #21558. I tried to update the expected results for WPT using "./mach update-wpt"; let me know if I got something wrong.

---
<!-- 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
- [x] These changes fix the second half of #21558

<!-- Either: -->
- [x] There are web-platform-tests tests for these changes
- [ ] These changes do not require tests because ___

<!-- 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/22648)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 10, 2019

💔 Test failed - linux-rel-css

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Jan 10, 2019

@collares collares force-pushed the collares:ChannelSplitterNode branch from ab01190 to 1614d1a Jan 10, 2019

@collares collares force-pushed the collares:ChannelSplitterNode branch from 1614d1a to 1209cf1 Jan 10, 2019

@collares

This comment has been minimized.

Copy link
Contributor Author

collares commented Jan 11, 2019

Thanks! I added the interface to the file.

By the way, what is the pinging etiquette/protocol in situations such as this one? Should I just wait, add a comment, or use IRC?

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Jan 11, 2019

Leave a comment or use IRC, your choice

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Jan 11, 2019

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 11, 2019

📌 Commit 1209cf1 has been approved by Manishearth

bors-servo added a commit that referenced this pull request Jan 12, 2019

Auto merge of #22648 - collares:ChannelSplitterNode, r=Manishearth
Implement DOM APIs for ChannelSplitterNode

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

Based on #21591. Fixes #21558. I tried to update the expected results for WPT using "./mach update-wpt"; let me know if I got something wrong.

---
<!-- 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
- [x] These changes fix the second half of #21558

<!-- Either: -->
- [x] There are web-platform-tests tests for these changes
- [ ] These changes do not require tests because ___

<!-- 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/22648)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 12, 2019

💔 Test failed - status-taskcluster

@collares

This comment has been minimized.

Copy link
Contributor Author

collares commented Jan 12, 2019

Ran 4402 tests finished in 1687.0 seconds.
  • 4401 ran as expected. 547 tests skipped.
  • 1 tests failed unexpectedly

Tests with unexpected results:
  ▶ FAIL [expected PASS] /css/CSS2/colors/color-142.xht
  └   → /css/CSS2/colors/color-142.xht 8fcd2853900c5150a8b614cfb9d661cacab3d874
/css/CSS2/colors/color-142-ref.xht 3e58793d8652fb373c53ccc19a963a632ca341d6
Testing 8fcd2853900c5150a8b614cfb9d661cacab3d874 == 3e58793d8652fb373c53ccc19a963a632ca341d6
@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Jan 12, 2019

@bors-servo retry

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Jan 12, 2019

Probably a new intermittent

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 12, 2019

💣 Failed to start rebuilding: Unknown error

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 12, 2019

⌛️ Testing commit 1209cf1 with merge 400e5ad...

bors-servo added a commit that referenced this pull request Jan 12, 2019

Auto merge of #22648 - collares:ChannelSplitterNode, r=Manishearth
Implement DOM APIs for ChannelSplitterNode

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

Based on #21591. Fixes #21558. I tried to update the expected results for WPT using "./mach update-wpt"; let me know if I got something wrong.

---
<!-- 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
- [x] These changes fix the second half of #21558

<!-- Either: -->
- [x] There are web-platform-tests tests for these changes
- [ ] These changes do not require tests because ___

<!-- 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/22648)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 12, 2019

💔 Test failed - mac-rel-wpt4

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Jan 12, 2019

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 12, 2019

⌛️ Testing commit 1209cf1 with merge c7cd1b8...

bors-servo added a commit that referenced this pull request Jan 12, 2019

Auto merge of #22648 - collares:ChannelSplitterNode, r=Manishearth
Implement DOM APIs for ChannelSplitterNode

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

Based on #21591. Fixes #21558. I tried to update the expected results for WPT using "./mach update-wpt"; let me know if I got something wrong.

---
<!-- 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
- [x] These changes fix the second half of #21558

<!-- Either: -->
- [x] There are web-platform-tests tests for these changes
- [ ] These changes do not require tests because ___

<!-- 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/22648)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 12, 2019

💔 Test failed - mac-rel-css1

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Jan 12, 2019

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 12, 2019

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 12, 2019

@bors-servo bors-servo referenced this pull request Jan 12, 2019

Merged

Implement formdata event #22660

4 of 4 tasks complete

@bors-servo bors-servo merged commit 1209cf1 into servo:master Jan 12, 2019

3 checks passed

Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details

@collares collares deleted the collares:ChannelSplitterNode branch Jan 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment