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

Jump to bottom

Continue to slim down the code for CSS properties #17122

Merged
merged 4 commits into from Jun 2, 2017
Merged

Continue to slim down the code for CSS properties #17122

merged 4 commits into from Jun 2, 2017

Conversation

nox
Copy link
Contributor

@nox nox commented Jun 1, 2017 •
edited by larsbergstrom

This change is Reviewable

@highfive
Copy link

highfive commented Jun 1, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/values/generics/text.rs, components/style/values/computed/text.rs, components/style/values/computed/mod.rs, components/style/properties/longhand/outline.mako.rs, components/style/values/generics/border.rs and 13 more
  • @KiChjang: components/script/dom/element.rs
  • @fitzgen: components/script/dom/element.rs
  • @emilio: components/style/values/generics/text.rs, ports/geckolib/glue.rs, components/style/values/computed/text.rs, components/style/values/computed/mod.rs, components/style/properties/longhand/outline.mako.rs and 14 more

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jun 1, 2017
@nox
Copy link
Contributor Author

nox commented Jun 1, 2017

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fe397e48463df279769b7208c172966cde05df26

emilio
emilio approved these changes Jun 1, 2017
View reviewed changes
Copy link
Member

@emilio emilio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me

components/style/values/computed/text.rs Outdated
/// A computed value for the `letter-spacing` property.
pub type LetterSpacing = Spacing<Length>;

/// A specified value for the `letter-spacing` property.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A computed value for the word-spacing property, right?

@wafflespeanut
Copy link
Contributor

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit fb6bc02 has been approved by wafflespeanut

@highfive highfive assigned wafflespeanut and unassigned Manishearth Jun 1, 2017
@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 Jun 1, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit fb6bc02 with merge 336d90b...

bors-servo pushed a commit that referenced this pull request Jun 1, 2017
Auto merge of #17122 - servo:derive-all-the-things, r=wafflespeanut
336d90b 
Continue to slim down the code for CSS properties

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17122)
<!-- Reviewable:end -->
@wafflespeanut
Copy link
Contributor

@bors-servo r-

Oh, didn't see that comment.

@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jun 1, 2017
@nox
Copy link
Contributor Author

nox commented Jun 1, 2017

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 2adb5fe has been approved by nox

@highfive highfive assigned nox and unassigned wafflespeanut Jun 1, 2017
@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 Jun 1, 2017
@nox
Copy link
Contributor Author

nox commented Jun 1, 2017

@bors-servo r=emilio

@bors-servo
Copy link
Contributor

💡 This pull request was already approved, no need to approve it again.

@bors-servo
Copy link
Contributor

📌 Commit 2adb5fe has been approved by emilio

@highfive highfive assigned emilio and unassigned nox Jun 1, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit 2adb5fe with merge 08c251f...

bors-servo pushed a commit that referenced this pull request Jun 1, 2017
Auto merge of #17122 - servo:derive-all-the-things, r=emilio
08c251f 
Continue to slim down the code for CSS properties

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17122)
<!-- Reviewable:end -->
@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jun 2, 2017
@nox
Copy link
Contributor Author

nox commented Jun 2, 2017

@bors-servo r=emilio

@bors-servo
Copy link
Contributor

📌 Commit c4e8ee1 has been approved by emilio

@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. S-needs-rebase There are merge conflict errors. labels Jun 2, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit c4e8ee1 with merge ac546ae...

bors-servo pushed a commit that referenced this pull request Jun 2, 2017
Auto merge of #17122 - servo:derive-all-the-things, r=emilio
ac546ae 
Continue to slim down the code for CSS properties

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17122)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - linux-rel-wpt

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jun 2, 2017
@nox
Copy link
Contributor Author

nox commented Jun 2, 2017 

  ▶ Unexpected subtest result in /cssom/serialize-values.html:
  └ PASS [expected FAIL] outline-width: thin

  ▶ Unexpected subtest result in /cssom/serialize-values.html:
  └ PASS [expected FAIL] outline-width: medium

  ▶ Unexpected subtest result in /cssom/serialize-values.html:
  └ PASS [expected FAIL] outline-width: thick

Nice.

@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Jun 2, 2017
@nox
Copy link
Contributor Author

nox commented Jun 2, 2017

@bors-servo r=emilio

@bors-servo
Copy link
Contributor

📌 Commit e58f541 has been approved by emilio

@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 Jun 2, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit e58f541 with merge 028908e...

bors-servo pushed a commit that referenced this pull request Jun 2, 2017
Auto merge of #17122 - servo:derive-all-the-things, r=emilio
028908e 
Continue to slim down the code for CSS properties

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17122)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: emilio
Pushing 028908e to master...

@bors-servo bors-servo merged commit e58f541 into master Jun 2, 2017
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jun 2, 2017
@SimonSapin SimonSapin deleted the derive-all-the-things branch June 2, 2017 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Manishearth Manishearth Manishearth left review comments

@emilio emilio emilio approved these changes

Assignees

@emilio emilio

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@nox @highfive @wafflespeanut @bors-servo @jdm @emilio @Manishearth