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

Derive more ToCss impls #17215

Merged
merged 2 commits into from Jun 9, 2017
Merged

Derive more ToCss impls #17215

merged 2 commits into from Jun 9, 2017
+54 −96

Conversation

@nox
Copy link
Member

nox commented Jun 7, 2017 •
edited by larsbergstrom

This change is Reviewable
@highfive
Copy link

highfive commented Jun 7, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/properties/gecko.mako.rs, components/style/properties/longhand/inherited_text.mako.rs, components/style/properties/longhand/pointing.mako.rs, components/style/counter_style/mod.rs, components/style/values/mod.rs and 8 more
  • @emilio: components/layout/display_list_builder.rs, components/style/properties/gecko.mako.rs, components/style/properties/longhand/inherited_text.mako.rs, components/style/properties/longhand/pointing.mako.rs, components/style/counter_style/mod.rs and 9 more
@highfive
Copy link

highfive commented Jun 7, 2017

warning Warning warning

  • These commits modify style and layout code, but no tests are modified. Please consider adding a test!
@nox
Copy link
Member Author

nox commented Jun 7, 2017

r? @emilio

https://treeherder.mozilla.org/#/jobs?repo=try&revision=51ea184b05fcca6b8c9d49aa46158db79048d587
@highfive highfive assigned emilio and unassigned asajeffrey Jun 7, 2017
@emilio
emilio approved these changes Jun 7, 2017
View changes
components/style_traits/values.rs
@@ -38,6 +38,20 @@ impl<'a, T> ToCss for &'a T where T: ToCss + ?Sized {
}
}

impl ToCss for str {

This comment has been minimized.

Sign in to view
@emilio

emilio Jun 7, 2017

Member

Please add a comment in the ToCss definition pointing to this, since it may be easy to misuse and end up serialising other stuff in a way it was not intended to.
components/style/properties/longhand/inherited_text.mako.rs
match *self {
computed_value::T::Keyword(ref keyword) => keyword.to_css(dest),
computed_value::T::None => dest.write_str("none"),
computed_value::T::String(ref string) => write!(dest, "\"{}\"", string),

This comment has been minimized.

Sign in to view
@emilio

emilio Jun 7, 2017

Member

Ugh
@nox nox force-pushed the derive-all-the-things branch from 7b28d3d to a5aa227 Jun 8, 2017
@nox
Copy link
Member Author

nox commented Jun 8, 2017

@bors-servo r=emilio
@bors-servo
Copy link
Contributor

bors-servo commented Jun 8, 2017

📌 Commit a5aa227 has been approved by emilio
@bors-servo
Copy link
Contributor

bors-servo commented Jun 8, 2017

⌛ Testing commit a5aa227 with merge 7dba361...
bors-servo added a commit that referenced this pull request Jun 8, 2017
@bors-servo
Auto merge of #17215 - servo:derive-all-the-things, r=emilio
Loading status checks…
7dba361 
Derive more ToCss impls

<!-- 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/17215)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 8, 2017

💔 Test failed - linux-dev
nox added 2 commits Jun 7, 2017
@nox
Implement ToCss for str and String
c8c6f34
@nox
Derive ToCss for cursor keywords
Loading status checks…
d55d726
@nox nox force-pushed the derive-all-the-things branch from a5aa227 to d55d726 Jun 8, 2017
@nox
Copy link
Member Author

nox commented Jun 8, 2017

@bors-servo r=emilio
@bors-servo
Copy link
Contributor

bors-servo commented Jun 8, 2017

📌 Commit d55d726 has been approved by emilio
@bors-servo
Copy link
Contributor

bors-servo commented Jun 8, 2017

⌛ Testing commit d55d726 with merge 62471af...
bors-servo added a commit that referenced this pull request Jun 8, 2017
@bors-servo
Auto merge of #17215 - servo:derive-all-the-things, r=emilio
Loading status checks…
62471af 
Derive more ToCss impls

<!-- 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/17215)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 8, 2017

💔 Test failed - mac-rel-wpt2
@nox
Copy link
Member Author

nox commented Jun 8, 2017

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

bors-servo commented Jun 8, 2017

⚡ Previous build results for 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-wpt3, mac-rel-wpt4, windows-msvc-dev are reusable. Rebuilding only mac-rel-wpt2...
@bors-servo
Copy link
Contributor

bors-servo commented Jun 8, 2017

💔 Test failed - mac-rel-wpt2
@nox
Copy link
Member Author

nox commented Jun 9, 2017

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

bors-servo commented Jun 9, 2017

⌛ Testing commit d55d726 with merge 1555f0f...
bors-servo added a commit that referenced this pull request Jun 9, 2017
@bors-servo
Auto merge of #17215 - servo:derive-all-the-things, r=emilio
Loading status checks…
1555f0f 
Derive more ToCss impls

<!-- 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/17215)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 9, 2017

💔 Test failed - linux-rel-css
@jdm
Copy link
Member

jdm commented Jun 9, 2017

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

bors-servo commented Jun 9, 2017

⚡ Previous build results for android, arm32, arm64, linux-dev, 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 are reusable. Rebuilding only linux-rel-css...
@bors-servo
Copy link
Contributor

bors-servo commented Jun 9, 2017

☀️ 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 1555f0f to master...
@bors-servo bors-servo merged commit d55d726 into master Jun 9, 2017
4 checks passed
4 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
dependency-ci Dependencies checked
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emilio emilio

Assignees

@emilio emilio

Labels
S-tests-failed
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
@nox @highfive @bors-servo @jdm @emilio @asajeffrey
You can’t perform that action at this time.