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

Update rust-selectors #6427

Merged
merged 1 commit into from Jun 21, 2015
Merged

Update rust-selectors #6427

merged 1 commit into from Jun 21, 2015

Conversation

@SimonSapin
Copy link
Member

SimonSapin commented Jun 19, 2015

servo/rust-selectors#30

r? @Ms2ger

This conflicts with the SpiderMonkey upgrade #6150. I’m happy to wait until that lands and rebase.

Review on Reviewable
@highfive
Copy link

highfive commented Jun 19, 2015

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Jun 19, 2015

Critic review: https://critic.hoppipolla.co.uk/r/5331

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.
@SimonSapin
Update rust-selectors
Loading status checks…
fc25397 
servo/rust-selectors#30
@SimonSapin SimonSapin force-pushed the selector-traits-refactor branch from d93bf5f to fc25397 Jun 20, 2015
@SimonSapin
Copy link
Member Author

SimonSapin commented Jun 20, 2015

Rebase, now that #6150 has landed. r? @Ms2ger
@Ms2ger
Copy link
Contributor

Ms2ger commented Jun 20, 2015

Would it make sense to implement those traits on Node rather than &'a Node?
@Ms2ger Ms2ger self-assigned this Jun 20, 2015
@SimonSapin
Copy link
Member Author

SimonSapin commented Jun 21, 2015

I tried that at first, but e.g. get_parent returns Self, not &'a Self, and I don’t think returning an owned/cloned Node is what we want. In Kuchiki on the other hand, the trait is implemented for NodeRef which contains an Rc, so the ownership of the strong reference is significant and changing the trait to return &'a Self wouldn’t work.
@Ms2ger
Copy link
Contributor

Ms2ger commented Jun 21, 2015

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Jun 21, 2015

📌 Commit fc25397 has been approved by Ms2ger
@bors-servo
Copy link
Contributor

bors-servo commented Jun 21, 2015

⌛ Testing commit fc25397 with merge 7021546...
bors-servo pushed a commit that referenced this pull request Jun 21, 2015
bors-servo
Auto merge of #6427 - servo:selector-traits-refactor, r=Ms2ger
7021546 
Update rust-selectors

servo/rust-selectors#30

r? @Ms2ger

This conflicts with the SpiderMonkey upgrade #6150. I’m happy to wait until that lands and rebase.

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

bors-servo commented Jun 21, 2015

💔 Test failed - linux2
@Ms2ger
Copy link
Contributor

Ms2ger commented Jun 21, 2015

@bors-servo retry
@bors-servo
Copy link
Contributor

bors-servo commented Jun 21, 2015

⌛ Testing commit fc25397 with merge c119b59...
bors-servo pushed a commit that referenced this pull request Jun 21, 2015
bors-servo
Auto merge of #6427 - servo:selector-traits-refactor, r=Ms2ger
Loading status checks…
c119b59 
Update rust-selectors

servo/rust-selectors#30

r? @Ms2ger

This conflicts with the SpiderMonkey upgrade #6150. I’m happy to wait until that lands and rebase.

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

bors-servo commented Jun 21, 2015

☀️ Test successful - android, gonk, linux1, linux2, linux3, mac1, mac2, mac3
@bors-servo bors-servo merged commit fc25397 into master Jun 21, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@Ms2ger Ms2ger deleted the selector-traits-refactor branch Jun 22, 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-answer
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@SimonSapin @highfive @hoppipolla-critic-bot @Ms2ger @bors-servo
You can’t perform that action at this time.