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

Changed return value of GetActiveCues #22730

Merged
merged 1 commit into from Jan 21, 2019
Merged

Changed return value of GetActiveCues #22730

merged 1 commit into from Jan 21, 2019

Conversation

@aditj
Copy link
Contributor

aditj commented Jan 18, 2019

Return an empty TextTrackCueList rather than Some inside the GetActiveCues.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #22706 (GitHub issue number if applicable)
  • There are tests for these changes OR
  • These changes do not require tests because ___

This change is Reviewable

@highfive
Copy link

highfive commented Jan 18, 2019

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 Jan 18, 2019

Heads up! This PR modifies the following files:

@highfive
Copy link

highfive commented Jan 18, 2019

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
@jdm jdm assigned ferjm and unassigned nox Jan 18, 2019
@@ -112,9 +112,7 @@ impl TextTrackMethods for TextTrack {

// https://html.spec.whatwg.org/multipage/#dom-texttrack-activecues
fn GetActiveCues(&self) -> Option<DomRoot<TextTrackCueList>> {
// XXX implement active cues logic
// https://github.com/servo/servo/issues/22314

This comment has been minimized.

Copy link
@ferjm

ferjm Jan 18, 2019

Member

This comment is still valid. Do not remove it, please.

@ferjm
ferjm approved these changes Jan 21, 2019
Copy link
Member

ferjm left a comment

Looks good. This is ready to merge after squashing the commits. Thanks!

@aditj
Copy link
Contributor Author

aditj commented Jan 21, 2019

Looks good. This is ready to merge after squashing the commits. Thanks!

Ya I was trying that , is there any way I can squash commits of a remote branch(which has not been edited locally) ?

@ferjm
Copy link
Member

ferjm commented Jan 21, 2019

git rebase -i HEAD~2 should open the interactive rebase tool in your default text editor. Substitute the pick word before the second commit by an s or squash, save and exit.

Return an empty TextTrackCueList rather than Some.

Added previously removed comments.
@aditj aditj force-pushed the aditj:patch-2 branch from 72a4f53 to df1119b Jan 21, 2019
@ferjm
Copy link
Member

ferjm commented Jan 21, 2019

@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented Jan 21, 2019

Trying commit df1119b with merge 69419b3...

bors-servo added a commit that referenced this pull request Jan 21, 2019
Changed return value of GetActiveCues

Return an empty TextTrackCueList rather than Some inside the GetActiveCues.

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

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [X] These changes fix #22706 (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] 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/22730)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 21, 2019

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
State: approved= try=True

@ferjm
Copy link
Member

ferjm commented Jan 21, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Jan 21, 2019

📌 Commit df1119b has been approved by ferjm

@bors-servo
Copy link
Contributor

bors-servo commented Jan 21, 2019

Testing commit df1119b with merge 8557e15...

bors-servo added a commit that referenced this pull request Jan 21, 2019
Changed return value of GetActiveCues

Return an empty TextTrackCueList rather than Some inside the GetActiveCues.

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

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [X] These changes fix #22706 (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] 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/22730)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 21, 2019

💔 Test failed - status-taskcluster

@ferjm
Copy link
Member

ferjm commented Jan 21, 2019

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Jan 21, 2019

💣 Failed to start rebuilding: Unknown error

@bors-servo
Copy link
Contributor

bors-servo commented Jan 21, 2019

Testing commit df1119b with merge 00148da...

bors-servo added a commit that referenced this pull request Jan 21, 2019
Changed return value of GetActiveCues

Return an empty TextTrackCueList rather than Some inside the GetActiveCues.

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

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [X] These changes fix #22706 (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] 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/22730)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 21, 2019

@bors-servo bors-servo merged commit df1119b into servo:master Jan 21, 2019
4 of 5 checks passed
4 of 5 checks passed
Travis CI - Pull Request Build Created
Details
Taskcluster (pull_request) TaskGroup: success
Details
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
@aditj aditj deleted the aditj:patch-2 branch Jan 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.