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

Implement Stringifier for Range #9796

Merged
merged 1 commit into from Mar 10, 2016
Merged

Implement Stringifier for Range #9796

merged 1 commit into from Mar 10, 2016

Conversation

@KiChjang
Copy link
Member

KiChjang commented Feb 28, 2016

Review on Reviewable
@highfive
Copy link

highfive commented Feb 28, 2016

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
@KiChjang
Copy link
Member Author

KiChjang commented Feb 28, 2016

@bors-servo try
@bors-servo
Copy link
Contributor

bors-servo commented Feb 28, 2016

⌛ Trying commit b2c16f9 with merge ed7955f...
bors-servo added a commit that referenced this pull request Feb 28, 2016
@bors-servo
Auto merge of #9796 - KiChjang:range-stringifier, r=<try>
ed7955f 
Implement Stringifier for Range

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9796)
<!-- Reviewable:end -->
@emilio
Copy link
Member

emilio commented Feb 28, 2016

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion.

components/script/dom/range.rs, line 871 [r1] (raw file):
I'm almost sure there's a missing paren here :P

Comments from the review on Reviewable.io
@KiChjang KiChjang force-pushed the KiChjang:range-stringifier branch from b2c16f9 to d00d5a9 Feb 28, 2016
@KiChjang
Copy link
Member Author

KiChjang commented Feb 28, 2016

@bors-servo try
@bors-servo
Copy link
Contributor

bors-servo commented Feb 28, 2016

⌛ Trying commit d00d5a9 with merge 0e667bb...
bors-servo added a commit that referenced this pull request Feb 28, 2016
@bors-servo
Auto merge of #9796 - KiChjang:range-stringifier, r=<try>
0e667bb 
Implement Stringifier for Range

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9796)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Feb 28, 2016

💔 Test failed - linux-dev
@nox
Copy link
Member

nox commented Feb 28, 2016

#9795
@KiChjang KiChjang force-pushed the KiChjang:range-stringifier branch from d00d5a9 to cabcce4 Feb 28, 2016
@KiChjang
Copy link
Member Author

KiChjang commented Feb 28, 2016

@bors-servo try
@bors-servo
Copy link
Contributor

bors-servo commented Feb 28, 2016

⌛ Trying commit cabcce4 with merge 39d6a68...
bors-servo added a commit that referenced this pull request Feb 28, 2016
@bors-servo
Auto merge of #9796 - KiChjang:range-stringifier, r=<try>
39d6a68 
Implement Stringifier for Range

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9796)
<!-- Reviewable:end -->
@KiChjang KiChjang force-pushed the KiChjang:range-stringifier branch from cabcce4 to 21babee Feb 28, 2016
@KiChjang
Copy link
Member Author

KiChjang commented Feb 28, 2016

@bors-servo try
@bors-servo
Copy link
Contributor

bors-servo commented Feb 28, 2016

⌛ Trying commit 21babee with merge 9ca3f16...
bors-servo added a commit that referenced this pull request Feb 28, 2016
@bors-servo
Auto merge of #9796 - KiChjang:range-stringifier, r=<try>
9ca3f16 
Implement Stringifier for Range

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9796)
<!-- Reviewable:end -->
@nox
Copy link
Member

nox commented Feb 28, 2016 

Tests with unexpected results:
  ▶ Unexpected subtest result in /dom/interfaces.html:
  └ PASS [expected FAIL] Range interface: stringifier

This needs many more tests, I think.
@bors-servo
Copy link
Contributor

bors-servo commented Feb 28, 2016

💔 Test failed - linux-rel
@jdm
Copy link
Member

jdm commented Mar 9, 2016

-S-awaiting-review -S-needs-tests +S-needs-code-changes

Reviewed 1 of 1 files at r4.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

tests/wpt/web-platform-tests/dom/ranges/Range-stringifier.html, line 17 [r4] (raw file):
Please use a separate test() for each testcase, and add a descriptive message (test(..., "message")).

tests/wpt/web-platform-tests/dom/ranges/Range-stringifier.html, line 20 [r4] (raw file):
Can we use a non-zero offset here?

Comments from the review on Reviewable.io
@KiChjang KiChjang force-pushed the KiChjang:range-stringifier branch from 6631eb2 to 3c541c3 Mar 9, 2016
@KiChjang
Implement Stringifier for Range
Loading status checks…
b32e859
@KiChjang KiChjang force-pushed the KiChjang:range-stringifier branch from 3c541c3 to b32e859 Mar 9, 2016
@KiChjang KiChjang closed this Mar 9, 2016
@KiChjang KiChjang reopened this Mar 9, 2016
@jdm
Copy link
Member

jdm commented Mar 9, 2016

@bors-servo: r+
Thanks!

Reviewed 1 of 1 files at r5.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks broke.

Comments from the review on Reviewable.io
@bors-servo
Copy link
Contributor

bors-servo commented Mar 9, 2016

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

bors-servo commented Mar 9, 2016

⌛ Testing commit b32e859 with merge 3e5a028...
bors-servo added a commit that referenced this pull request Mar 9, 2016
@bors-servo
Auto merge of #9796 - KiChjang:range-stringifier, r=jdm
Loading status checks…
3e5a028 
Implement Stringifier for Range

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9796)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Mar 9, 2016

💔 Test failed - status-appveyor
@KiChjang
Copy link
Member Author

KiChjang commented Mar 10, 2016

@bors-servo retry

  • larsbors stopped the build
@bors-servo
Copy link
Contributor

bors-servo commented Mar 10, 2016

⌛ Testing commit b32e859 with merge fa93d3f...
bors-servo added a commit that referenced this pull request Mar 10, 2016
@bors-servo
Auto merge of #9796 - KiChjang:range-stringifier, r=jdm
Loading status checks…
fa93d3f 
Implement Stringifier for Range

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9796)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Mar 10, 2016

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, status-appveyor
@bors-servo bors-servo merged commit b32e859 into servo:master Mar 10, 2016
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/appveyor/pr AppVeyor build cancelled
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@KiChjang KiChjang deleted the KiChjang:range-stringifier branch Mar 10, 2016
@mbrubeck mbrubeck mentioned this pull request May 25, 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-awaiting-merge
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
@KiChjang @highfive @bors-servo @emilio @nox @jdm
You can’t perform that action at this time.