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

Avoid some rooting in parsing. #8935

Merged
merged 1 commit into from Dec 11, 2015

Conversation

Projects
None yet
5 participants
@Ms2ger
Contributor

Ms2ger commented Dec 11, 2015

Review on Reviewable

@nox

This comment has been minimized.

Member

nox commented Dec 11, 2015

-S-awaiting-review +S-awaiting-answer


Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


components/script/parse/html.rs, line 43 [r1] (raw file):
How come this doesn't need #[allow(unrooted_must_root)]? There is a JS<Node> passed by value in there.


Comments from the review on Reviewable.io

@nox nox self-assigned this Dec 11, 2015

@Ms2ger

This comment has been minimized.

Contributor

Ms2ger commented Dec 11, 2015

@Manishearth

This comment has been minimized.

Member

Manishearth commented Dec 11, 2015

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Dec 11, 2015

📌 Commit 14acf16 has been approved by Manishearth

@Manishearth

This comment has been minimized.

Member

Manishearth commented Dec 11, 2015

@Manishearth

This comment has been minimized.

Member

Manishearth commented Dec 11, 2015

Review status: all files reviewed at latest revision, 1 unresolved discussion.


components/script/parse/html.rs, line 43 [r1] (raw file):
The lint is turned off in this file because we do far worse.

But. I wonder if we can pass in an &JS. It's safe at the moment though, so r=me if we don't want to.


Comments from the review on Reviewable.io

@Ms2ger

This comment has been minimized.

Contributor

Ms2ger commented Dec 11, 2015

I don't think we can.

@Manishearth

This comment has been minimized.

Member

Manishearth commented Dec 11, 2015

'kay

@bors-servo r+

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Dec 11, 2015

📌 Commit 14acf16 has been approved by Manishearth

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Dec 11, 2015

⌛️ Testing commit 14acf16 with merge 2a416e7...

bors-servo added a commit that referenced this pull request Dec 11, 2015

Auto merge of #8935 - Ms2ger:get_or_create, r=Manishearth
Avoid some rooting in parsing.

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

@nox nox assigned Manishearth and unassigned nox Dec 11, 2015

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Dec 11, 2015

@bors-servo bors-servo merged commit 14acf16 into servo:master Dec 11, 2015

2 of 3 checks passed

code-review/reviewable Review in progress: all files reviewed, 1 unresolved discussion
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@Ms2ger Ms2ger deleted the Ms2ger:get_or_create branch Dec 18, 2015

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