Skip to content
This repository was archived by the owner on Nov 12, 2022. It is now read-only.

Make Rooted do its job.#326

Merged
bors-servo merged 2 commits intomasterfrom
rooting
Jan 13, 2017
Merged

Make Rooted do its job.#326
bors-servo merged 2 commits intomasterfrom
rooting

Conversation

@Ms2ger
Copy link
Copy Markdown
Contributor

@Ms2ger Ms2ger commented Jan 13, 2017

This change is Reviewable

@Ms2ger
Copy link
Copy Markdown
Contributor Author

Ms2ger commented Jan 13, 2017

My main fear is that the two out-of-line calls will be terrible for performance. I don't suppose we use Rooted terribly often…

@Ms2ger Ms2ger force-pushed the rooting branch 2 times, most recently from ef6bcbd to ce5c85c Compare January 13, 2017 13:12
@Ms2ger
Copy link
Copy Markdown
Contributor Author

Ms2ger commented Jan 13, 2017

@jdm can you check the fixup commit? I'm not sure if the additional unsafety is worth it.

@jdm
Copy link
Copy Markdown
Member

jdm commented Jan 13, 2017

As mentioned on IRC, I think we should follow jonco's suggestion from servo/servo#13096 (comment) rather than pursue the patch I wrote in that issue.

@jdm
Copy link
Copy Markdown
Member

jdm commented Jan 13, 2017

I'm glad to see the test, though!

@jdm
Copy link
Copy Markdown
Member

jdm commented Jan 13, 2017

@bors-servo: r+
Nice.

@bors-servo
Copy link
Copy Markdown
Contributor

📌 Commit c8f4cab has been approved by jdm

@bors-servo
Copy link
Copy Markdown
Contributor

⌛ Testing commit c8f4cab with merge 568eccb...

bors-servo pushed a commit that referenced this pull request Jan 13, 2017
@bors-servo
Copy link
Copy Markdown
Contributor

☀️ Test successful - status-appveyor, status-travis

@bors-servo bors-servo merged commit c8f4cab into master Jan 13, 2017
@Ms2ger Ms2ger deleted the rooting branch January 13, 2017 16:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants