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.

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 to tip #8142

Closed
wants to merge 1 commit into from
Closed

Conversation

@bholley
Copy link
Contributor

bholley commented Oct 21, 2015

Need this in order to make additional rust-selector changes for Issue#6942.

Review on Reviewable

@highfive
Copy link

highfive commented Oct 21, 2015

warning Warning warning

  • These commits modify layout code, but no reftests are modified. Please consider adding a reftest!
@@ -470,6 +470,12 @@ impl<'le> ::selectors::Element for LayoutElement<'le> {
self.element.get_event_state_for_layout().contains(element::IN_FOCUS_STATE)
}

// FIXME(#7720)
#[inline]

This comment has been minimized.

@Manishearth

Manishearth Oct 21, 2015

Member

No need for the hint; LLVM can figure this out on its own and inline hints slow compile times in most cases

(brson did a study on the tradeoffs of #[inline], in almost all cases it didn't help)

@bholley bholley force-pushed the bholley:update_selectors branch from cbca1a8 to c4cd742 Oct 21, 2015
@bholley
Copy link
Contributor Author

bholley commented Oct 21, 2015

@Manishearth
Copy link
Member

Manishearth commented Oct 21, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Oct 21, 2015

📌 Commit c4cd742 has been approved by Manishearth

@bors-servo
Copy link
Contributor

bors-servo commented Oct 21, 2015

Testing commit c4cd742 with merge e226032...

bors-servo pushed a commit that referenced this pull request Oct 21, 2015
Update rust-selectors to tip

Need this in order to make additional rust-selector changes for Issue#6942.

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

bors-servo commented Oct 21, 2015

💔 Test failed - mac-dev-ref-unit

@Manishearth
Copy link
Member

Manishearth commented Oct 21, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Oct 21, 2015

Testing commit c4cd742 with merge 369c445...

bors-servo pushed a commit that referenced this pull request Oct 21, 2015
Update rust-selectors to tip

Need this in order to make additional rust-selector changes for Issue#6942.

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

bors-servo commented Oct 21, 2015

💔 Test failed - linux-rel

@Manishearth
Copy link
Member

Manishearth commented Oct 21, 2015

Ah, a WPT failure.

Could you run ./mach update-wpt wpt.log, where wpt.log is http://build.servo.org/builders/linux-rel/builds/625/steps/shell_1/logs/test-wpt.log/text, downloaded locally.

This should create a new commit.

@bholley
Copy link
Contributor Author

bholley commented Oct 22, 2015

@nox
Copy link
Member

nox commented Oct 22, 2015

@bholley Could you squash the two commits together? If your first commit ends up in a bisection, there will be unintended failures.

@nox nox removed the S-awaiting-review label Oct 22, 2015
@Ms2ger
Copy link
Contributor

Ms2ger commented Oct 22, 2015

This is wring per http://www.w3.org/TR/CSS/#partial; it would be better to just push a branch based on servo/rust-selectors@1e1e44c and update to that.

@Manishearth
Copy link
Member

Manishearth commented Oct 22, 2015

If it's a transient FIXME I think it's fine.

@bholley bholley force-pushed the bholley:update_selectors branch from 62745b2 to 81306a0 Oct 22, 2015
@Manishearth
Copy link
Member

Manishearth commented Oct 22, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Oct 22, 2015

📌 Commit 81306a0 has been approved by Manishearth

@bholley
Copy link
Contributor Author

bholley commented Oct 22, 2015

Ok then, backing it out per @jdm's request in servo/rust-selectors#54

@bholley bholley closed this Oct 22, 2015
@bholley bholley deleted the bholley:update_selectors branch Oct 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.