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

Update js. #8428

Merged
merged 3 commits into from Nov 12, 2015
Merged

Update js. #8428

merged 3 commits into from Nov 12, 2015

Conversation

Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented Nov 9, 2015

Review on Reviewable

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 9, 2015
@highfive
Copy link

highfive commented Nov 9, 2015

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

@Ms2ger
Copy link
Contributor Author

Ms2ger commented Nov 9, 2015

Depends on servo/rust-mozjs#213

@Ms2ger Ms2ger force-pushed the conversions branch 4 times, most recently from 0a945db to 9e8b644 Compare November 10, 2015 09:24
@Manishearth Manishearth self-assigned this Nov 10, 2015
@Manishearth
Copy link
Member

Reviewed 3 of 3 files at r1, 5 of 5 files at r2, 12 of 15 files at r3.
Review status: 17 of 20 files reviewed at latest revision, 2 unresolved discussions.

components/script/devtools.rs, line 31 [r3] (raw file):
Probably should have a comment here explaining why this block is safe, since it's pretty large.

components/script/dom/websocket.rs, line 573 [r3] (raw file):
ditto

Comments from the review on Reviewable.io

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #8197) 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 Nov 11, 2015
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Nov 11, 2015

Depends on servo/rust-mozjs#217

@Ms2ger Ms2ger added S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged. and removed S-awaiting-review There is new code that needs to be reviewed. S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged. S-needs-rebase There are merge conflict errors. labels Nov 11, 2015
@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #8412) 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 Nov 11, 2015
@Manishearth
Copy link
Member

Reviewed 3 of 15 files at r3, 5 of 5 files at r4.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

Comments from the review on Reviewable.io

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 12, 2015
@Ms2ger Ms2ger removed S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged. S-needs-rebase There are merge conflict errors. labels Nov 12, 2015
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Nov 12, 2015

In both cases, the explanation is basically "because window.get_cx() returns a valid pointer".

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

💔 Test failed - mac-rel-wpt

@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 Nov 12, 2015
@jdm
Copy link
Member

jdm commented Nov 12, 2015

@bors-servo: retry

@bors-servo
Copy link
Contributor

⚡ Previous build results for android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css are reusable. Rebuilding only mac-rel-wpt...

@bors-servo
Copy link
Contributor

💔 Test failed - mac-rel-wpt

@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 Nov 12, 2015
@jdm
Copy link
Member

jdm commented Nov 12, 2015

@bors-servo: retry

@bors-servo
Copy link
Contributor

⚡ Previous build results for android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css are reusable. Rebuilding only mac-rel-wpt...

@bors-servo
Copy link
Contributor

💔 Test failed - mac-rel-wpt

@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 Nov 12, 2015
@jdm
Copy link
Member

jdm commented Nov 12, 2015

@bors-servo: retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 6d2ae85 with merge b408820...

bors-servo pushed a commit that referenced this pull request Nov 12, 2015
Auto merge of #8428 - Ms2ger:conversions, r=Manishearth
b408820 
Update js.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8428)
<!-- 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-tests-failed The changes caused existing tests to fail. labels Nov 12, 2015
@bors-servo
Copy link
Contributor

💔 Test failed - android

@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 Nov 12, 2015
@jdm
Copy link
Member

jdm commented Nov 12, 2015

@bors-servo: retry

@bors-servo
Copy link
Contributor

⚡ Previous build results for gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt are reusable. Rebuilding only android...

@bors-servo
Copy link
Contributor

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt

@bors-servo bors-servo merged commit 6d2ae85 into servo:master Nov 12, 2015
@bors-servo bors-servo mentioned this pull request Nov 12, 2015
Merged
@Ms2ger Ms2ger deleted the conversions branch November 14, 2015 19:20
@SimonSapin SimonSapin removed the S-tests-failed The changes caused existing tests to fail. label Jan 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Manishearth Manishearth

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Ms2ger @highfive @Manishearth @bors-servo @jdm @SimonSapin