Use more generics and remove more code from mako files by nox · Pull Request #17186 · servo/servo

nox · 2017-06-06T15:21:10Z

This change is

highfive · 2017-06-06T15:21:14Z

Heads up! This PR modifies the following files:

@bholley: components/style/properties/gecko.mako.rs, components/style/values/specified/gecko.rs, components/style/values/generics/box.rs, components/style/values/specified/mod.rs, components/style/values/computed/mod.rs and 5 more
@emilio: components/style/properties/gecko.mako.rs, components/style/values/specified/gecko.rs, components/style/values/generics/box.rs, components/style/values/specified/mod.rs, components/style/values/computed/mod.rs and 5 more

highfive · 2017-06-06T15:21:15Z

Warning

These commits modify style code, but no tests are modified. Please consider adding a test!

nox · 2017-06-06T15:21:25Z

r? @emilio
@bors-servo try
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e83db3a822bb35d19c3820cddeee398118a46f8a

bors-servo · 2017-06-06T15:21:34Z

⌛ Trying commit a9cb635 with merge 4576cc9...

Use more generics and remove more code from mako files

emilio · 2017-06-06T15:30:34Z

components/style/values/generics/box.rs

+
+#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+#[derive(Clone, Copy, Debug, HasViewportPercentage, PartialEq, ToComputedValue, ToCss)]
+pub enum VerticalAlign<LengthOrPercentage>


emilio · 2017-06-06T15:31:38Z

components/style/values/generics/gecko.rs

+/// A generic value for scroll snap points.
+#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+#[derive(Clone, Copy, Debug, HasViewportPercentage, PartialEq, ToComputedValue)]
+pub enum ScrollSnapPoint<LengthOrPercentage> {


This is in a gecko-only module, so no need for a cfg_attr(feature = "servo"...). Though any reason why compiling it for both hurts?

bors-servo · 2017-06-06T15:45:58Z

💔 Test failed - windows-msvc-dev

jdm · 2017-06-06T15:48:51Z

   Compiling style v0.0.1 (file:///C:/buildbot/slave/windows-msvc-dev/build/components/style)
error[E0432]: unresolved import `values::specified::ScrollSnapPoint`
     --> c:\buildbot\slave\windows-msvc-dev\build\target\debug\build\style-56fbb21baf6b3c7d\out/properties.rs:16359:17
      |
16359 |         pub use values::specified::ScrollSnapPoint as SpecifiedValue;
      |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `ScrollSnapPoint` in `values::specified`

error[E0432]: unresolved import `values::computed::ScrollSnapPoint`
     --> c:\buildbot\slave\windows-msvc-dev\build\target\debug\build\style-56fbb21baf6b3c7d\out/properties.rs:16361:21
      |
16361 |             pub use values::computed::ScrollSnapPoint as T;
      |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `ScrollSnapPoint` in `values::computed`

error[E0432]: unresolved import `values::specified::ScrollSnapPoint`
     --> c:\buildbot\slave\windows-msvc-dev\build\target\debug\build\style-56fbb21baf6b3c7d\out/properties.rs:16521:17
      |
16521 |         pub use values::specified::ScrollSnapPoint as SpecifiedValue;
      |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `ScrollSnapPoint` in `values::specified`

error[E0432]: unresolved import `values::computed::ScrollSnapPoint`
     --> c:\buildbot\slave\windows-msvc-dev\build\target\debug\build\style-56fbb21baf6b3c7d\out/properties.rs:16523:21
      |
16523 |             pub use values::computed::ScrollSnapPoint as T;
      |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `ScrollSnapPoint` in `values::computed`

error[E0433]: failed to resolve. Could not find `ScrollSnapPoint` in `computed`
     --> c:\buildbot\slave\windows-msvc-dev\build\target\debug\build\style-56fbb21baf6b3c7d\out/properties.rs:16363:69
      |
16363 |         #[inline] pub fn get_initial_value() -> computed_value::T { computed::ScrollSnapPoint::none() }
      |                                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Could not find `ScrollSnapPoint` in `computed`

error[E0433]: failed to resolve. Could not find `ScrollSnapPoint` in `specified`
     --> c:\buildbot\slave\windows-msvc-dev\build\target\debug\build\style-56fbb21baf6b3c7d\out/properties.rs:16369:13
      |
16369 |             specified::ScrollSnapPoint::parse(context, input)
      |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Could not find `ScrollSnapPoint` in `specified`

error[E0433]: failed to resolve. Could not find `ScrollSnapPoint` in `computed`
     --> c:\buildbot\slave\windows-msvc-dev\build\target\debug\build\style-56fbb21baf6b3c7d\out/properties.rs:16525:69
      |
16525 |         #[inline] pub fn get_initial_value() -> computed_value::T { computed::ScrollSnapPoint::none() }
      |                                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Could not find `ScrollSnapPoint` in `computed`

error[E0433]: failed to resolve. Could not find `ScrollSnapPoint` in `specified`
     --> c:\buildbot\slave\windows-msvc-dev\build\target\debug\build\style-56fbb21baf6b3c7d\out/properties.rs:16531:13
      |
16531 |             specified::ScrollSnapPoint::parse(context, input)
      |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Could not find `ScrollSnapPoint` in `specified`

error: type `properties::DeclaredValue<'_, _>` cannot be dereferenced
     --> c:\buildbot\slave\windows-msvc-dev\build\target\debug\build\style-56fbb21baf6b3c7d\out/properties.rs:16407:31
      |
16407 |                         match *value {
      |                               ^^^^^^

error: type `properties::DeclaredValue<'_, _>` cannot be dereferenced
     --> c:\buildbot\slave\windows-msvc-dev\build\target\debug\build\style-56fbb21baf6b3c7d\out/properties.rs:16569:31
      |
16569 |                         match *value {
      |                               ^^^^^^

error: aborting due to previous error(s)

error: Could not compile `style`.

emilio

r=me

emilio · 2017-06-06T17:26:01Z

components/style/values/specified/gecko.rs

+            return Ok(GenericScrollSnapPoint::None);
+        }
+        input.expect_function_matching("repeat")?;
+        let length = input.parse_nested_block(|i|


nit: Wrap on braces?

nox · 2017-06-06T17:28:23Z

@bors-servo r=emilio

bors-servo · 2017-06-06T17:28:23Z

📌 Commit 195e98e has been approved by emilio

bors-servo · 2017-06-06T17:28:28Z

⌛ Testing commit 195e98e with merge 80488c4...

Use more generics and remove more code from mako files  This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17186)

bors-servo · 2017-06-06T18:33:04Z

☀️ 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 80488c4 to master...

nox added 3 commits June 6, 2017 14:35

Use predefined_type for transition-property

65356b1

Use predefined_type for animation-timing-function

8912a59

Use predefined_type for transition-delay

bc1eef8

highfive assigned jdm Jun 6, 2017

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jun 6, 2017

highfive assigned emilio and unassigned jdm Jun 6, 2017

bors-servo pushed a commit that referenced this pull request Jun 6, 2017

Auto merge of #17186 - servo:derive-all-the-things, r=<try>

4576cc9

Use more generics and remove more code from mako files

emilio reviewed Jun 6, 2017

View reviewed changes

highfive added the S-tests-failed The changes caused existing tests to fail. label Jun 6, 2017

nox force-pushed the derive-all-the-things branch from a9cb635 to 25931f9 Compare June 6, 2017 17:14

highfive removed the S-tests-failed The changes caused existing tests to fail. label Jun 6, 2017

emilio approved these changes Jun 6, 2017

View reviewed changes

Use generics for scroll-snap-points-*

195e98e

nox force-pushed the derive-all-the-things branch from 25931f9 to 195e98e Compare June 6, 2017 17:27

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 6, 2017

bors-servo mentioned this pull request Jun 6, 2017

Bump euclid to 0.14.x. #17184

Merged

5 tasks

bors-servo merged commit 195e98e into master Jun 6, 2017

highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jun 6, 2017

Uh oh!

Conversation

nox commented Jun 6, 2017 • edited by larsbergstrom Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

highfive commented Jun 6, 2017

Uh oh!

highfive commented Jun 6, 2017

Uh oh!

nox commented Jun 6, 2017

Uh oh!

bors-servo commented Jun 6, 2017

Uh oh!

emilio Jun 6, 2017

Choose a reason for hiding this comment

Uh oh!

emilio Jun 6, 2017

Choose a reason for hiding this comment

Uh oh!

bors-servo commented Jun 6, 2017

Uh oh!

jdm commented Jun 6, 2017

Uh oh!

emilio left a comment

Choose a reason for hiding this comment

Uh oh!

emilio Jun 6, 2017

Choose a reason for hiding this comment

Uh oh!

nox Jun 6, 2017

Choose a reason for hiding this comment

Uh oh!

nox commented Jun 6, 2017

Uh oh!

bors-servo commented Jun 6, 2017

Uh oh!

bors-servo commented Jun 6, 2017

Uh oh!

bors-servo commented Jun 6, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nox commented Jun 6, 2017 •

edited by larsbergstrom

Loading