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

Generalize the layout wrapper layer #8595

Merged
merged 3 commits into from Nov 19, 2015
Merged

Generalize the layout wrapper layer #8595

merged 3 commits into from Nov 19, 2015

Conversation

bholley
Copy link
Contributor

@bholley bholley commented Nov 19, 2015

There's still more refactoring to do, but this is the core stuff that's most likely to bitrot.

Review on Reviewable

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

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify layout code, but no reftests are modified. Please consider adding a reftest!

@pcwalton
Copy link
Contributor

Looks good modulo a couple of suggested uses of where clauses.

@jdm jdm added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Nov 19, 2015
@bholley
Hoist exported wrapper functionality into a family of traits.
54f2700 
All the existing code still uses the concrete implementations, so this
shouldn't impact the generated code at all.
@bholley
Prefix concrete types with 'Servo'.
9dd45b9
@bholley
Rename Layout*Trait to Layout*.
2f6e949 
The next step will be to remote the Servo-specificity of the layout code we want
to share by making it operate generically on Layout*.
@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Nov 19, 2015
@pcwalton
Copy link
Contributor

@bors-servo: r+

@bors-servo
Copy link
Contributor

📌 Commit 2f6e949 has been approved by pcwalton

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Nov 19, 2015
@bors-servo
Copy link
Contributor

⌛ Testing commit 2f6e949 with merge a5babb8...

bors-servo pushed a commit that referenced this pull request Nov 19, 2015
Auto merge of #8595 - bholley:generalize_wrappers, r=pcwalton
a5babb8 
Generalize the layout wrapper layer

There's still more refactoring to do, but this is the core stuff that's most likely to bitrot.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8595)
<!-- Reviewable:end -->
@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 2f6e949 into servo:master Nov 19, 2015
@SimonSapin SimonSapin mentioned this pull request Nov 20, 2015
Closed
@bholley bholley mentioned this pull request Nov 21, 2015
Merged
@bholley bholley deleted the generalize_wrappers branch October 30, 2016 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@pcwalton pcwalton

Labels
S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@bholley @highfive @pcwalton @bors-servo @jdm