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

Remove some redundant animation interpolation code #17157

Merged
merged 2 commits into from Jun 5, 2017

Conversation

upsuper
Copy link
Contributor

@upsuper upsuper commented Jun 5, 2017

This change is Reviewable

So that we can remove redundant interpolation code for corresponding
computed types.
@highfive
Copy link

highfive commented Jun 5, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/properties/helpers/animated_properties.mako.rs
  • @emilio: components/style/properties/helpers/animated_properties.mako.rs

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

upsuper commented Jun 5, 2017

Not sure who should review this... probably @hiikezoe or @birtles

@upsuper
Copy link
Contributor Author

upsuper commented Jun 5, 2017

r? @hiikezoe

@upsuper
Copy link
Contributor Author

upsuper commented Jun 5, 2017

@bors-servo try

@bors-servo
Copy link
Contributor

⌛ Trying commit 61eb7e0 with merge ef028be12a301257d01e22d5328ce80eb8c52f00...

@hiikezoe
Copy link
Contributor

hiikezoe commented Jun 5, 2017

Reviewed 1 of 1 files at r1.
Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion.


components/style/properties/helpers/animated_properties.mako.rs, line 365 at r2 (raw file):

% if not prop.is_animatable_with_computed_value:
                        let value: longhands::${prop.ident}::computed_value::T = value.into();
                    % endif

What I couldn't find initially was that the corresponding part of this change for stylo. Yeah, it's done in the first commit. Rust trait magic. Great!


Comments from Reviewable

@hiikezoe
Copy link
Contributor

hiikezoe commented Jun 5, 2017

Awesome! Thank you Xidorn!
r=me with a try for stylo.

@upsuper
Copy link
Contributor Author

upsuper commented Jun 5, 2017

@bors-servo
Copy link
Contributor

💔 Test failed - linux-rel-wpt

@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label Jun 5, 2017
@upsuper
Copy link
Contributor Author

upsuper commented Jun 5, 2017

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Trying commit 61eb7e0 with merge 67eaae8...

bors-servo pushed a commit that referenced this pull request Jun 5, 2017
Remove some redundant animation interpolation code

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

💔 Test failed - linux-rel-wpt

@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label Jun 5, 2017
@upsuper
Copy link
Contributor Author

upsuper commented Jun 5, 2017

@upsuper
Copy link
Contributor Author

upsuper commented Jun 5, 2017

@bors-servo r=hiikezoe

@bors-servo
Copy link
Contributor

📌 Commit 61eb7e0 has been approved by hiikezoe

@highfive highfive removed S-awaiting-review There is new code that needs to be reviewed. S-tests-failed The changes caused existing tests to fail. labels Jun 5, 2017
@bors-servo
Copy link
Contributor

⚡ Previous build results for android, arm32, arm64, linux-dev, linux-rel-css, mac-dev-unit, mac-rel-css1, mac-rel-wpt1, mac-rel-wpt4, windows-msvc-dev are reusable. Rebuilding only linux-rel-wpt, mac-rel-css2, mac-rel-wpt2, mac-rel-wpt3...

@highfive highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jun 5, 2017
@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: hiikezoe
Pushing 67eaae8 to master...

@bors-servo bors-servo merged commit 61eb7e0 into servo:master Jun 5, 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 5, 2017
@upsuper upsuper deleted the animation-type branch June 5, 2017 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants