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.

Sign up for GitHub

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

Jump to bottom

Implement HTMLTableElement#createTBody #7089

Merged
merged 1 commit into from Aug 9, 2015
Merged

Implement HTMLTableElement#createTBody #7089

merged 1 commit into from Aug 9, 2015

Conversation

dzbarsky
Copy link
Contributor

@dzbarsky dzbarsky commented Aug 8, 2015

Review on Reviewable

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Aug 8, 2015
@dzbarsky dzbarsky mentioned this pull request Aug 8, 2015
Closed
@Ms2ger Ms2ger added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Aug 8, 2015
@Ms2ger
Copy link
Contributor

Ms2ger commented Aug 8, 2015

Thank you for your PR.

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

Reviewed 4 of 4 files at r1.
Review status: all files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.

components/script/dom/htmltableelement.rs, line 92 [r1] (raw file):
Feel free to make this return HTMLTableSectionElement, even though that's not what the spec says.

components/script/dom/htmltableelement.rs, line 101 [r1] (raw file):
Indentation is weird; maybe brace the closure.

components/script/dom/htmltableelement.rs, line 103 [r1] (raw file):
Maybe use and_then.

components/script/dom/htmltableelement.rs, line 106 [r1] (raw file):
Just r() (on RootedReference).

Comments from the review on Reviewable.io

@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Aug 8, 2015
@dzbarsky
Copy link
Contributor Author

dzbarsky commented Aug 8, 2015

Review status: 2 of 4 files reviewed at latest revision, 4 unresolved discussions, some commit checks pending.

components/script/dom/htmltableelement.rs, line 106 [r1] (raw file):
Oh I thought that was gone after smup. Looks like it's back or something.

Comments from the review on Reviewable.io

@Ms2ger Ms2ger added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Aug 9, 2015
@Ms2ger
Copy link
Contributor

Ms2ger commented Aug 9, 2015

-S-awaiting-review +S-awaiting-merge

@bors-servo r+

Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.

Comments from the review on Reviewable.io

@bors-servo
Copy link
Contributor

📌 Commit 992a1e1 has been approved by Ms2ger

@bors-servo
Copy link
Contributor

⌛ Testing commit 992a1e1 with merge 454b2a7...

@highfive highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Aug 9, 2015
bors-servo pushed a commit that referenced this pull request Aug 9, 2015
Auto merge of #7089 - dzbarsky:createtbody, r=Ms2ger
454b2a7 
Implement HTMLTableElement#createTBody



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

💔 Test failed - linux2

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Aug 9, 2015
@Ms2ger
Copy link
Contributor

Ms2ger commented Aug 9, 2015 

/html/dom/interfaces.html
-------------------------
PASS expected FAIL HTMLTableElement interface: operation createTBody()
PASS expected FAIL HTMLTableElement interface: document.createElement("table") must inherit property "createTBody" with the proper type (10)

@bors-servo
Copy link
Contributor

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

@highfive highfive added S-needs-rebase There are merge conflict errors. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Aug 9, 2015
@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Aug 9, 2015
@Ms2ger
Copy link
Contributor

Ms2ger commented Aug 9, 2015

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 54adae2 has been approved by Ms2ger

@bors-servo
Copy link
Contributor

⌛ Testing commit 54adae2 with merge 44d93bc...

bors-servo pushed a commit that referenced this pull request Aug 9, 2015
Auto merge of #7089 - dzbarsky:createtbody, r=Ms2ger
44d93bc 
Implement HTMLTableElement#createTBody



<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7089)
<!-- Reviewable:end -->
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. S-needs-rebase There are merge conflict errors. labels Aug 9, 2015
@bors-servo
Copy link
Contributor

☀️ Test successful - android, gonk, linux1, linux2, mac1, mac2, mac3

@bors-servo bors-servo merged commit 54adae2 into servo:master Aug 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Ms2ger Ms2ger

Labels
S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@dzbarsky @Ms2ger @bors-servo @highfive