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

Support plain number-typed properties in geckolib #10957

Merged
merged 5 commits into from May 3, 2016
Merged

Conversation

@heycam
Copy link
Member

heycam commented May 2, 2016

r? @bholley


This change is Reviewable

@highfive
Copy link

highfive commented May 2, 2016

Heads up! This PR modifies the following files:

  • @bholley: components/style/properties/longhand/effects.mako.rs, components/style/properties/data.py, components/style/values.rs, components/style/properties/helpers.mako.rs, components/style/properties/longhand/position.mako.rs, components/style/properties/longhand/svg_inherited.mako.rs, components/style/properties/longhand/svg.mako.rs, components/style/properties/longhand/xul.mako.rs
@highfive
Copy link

highfive commented May 2, 2016

warning Warning warning

  • These commits modify style code, but no tests are modified. Please consider adding a test!
fn parse_with_minimum(input: &mut Parser, min: CSSFloat) -> Result<Number, ()> {
match parse_number(input) {
Ok(value) if value < min => Err(()),
value => value.map(Number),

This comment has been minimized.

Copy link
@bholley

bholley May 2, 2016

Contributor

Using a newtype as a function in this way seems a bit weird, but maybe it's a totally acceptable pattern. Can you verify that this is acceptable Rust? (No need to block landing on it)

This comment has been minimized.

Copy link
@heycam

heycam May 2, 2016

Author Member

There are a couple of other similar uses in this file – mapping to Au and Percentage – and a bunch more mapping to enum constructors, which is not so different.

@bholley
Copy link
Contributor

bholley commented May 2, 2016

Nice!

@bors-servo r=bholley

Don't forget to update the spreadsheet!

@bors-servo
Copy link
Contributor

bors-servo commented May 2, 2016

📌 Commit 22513a6 has been approved by bholley

@bholley
Copy link
Contributor

bholley commented May 2, 2016

Oh wait, tidy problem. @bor-servo r-

@bholley
Copy link
Contributor

bholley commented May 2, 2016

@bors-servo delegate+

I'd recommend running ./mach test-tidy before pushing PRs.

@bors-servo
Copy link
Contributor

bors-servo commented May 2, 2016

✌️ @heycam can now approve this pull request

@bors-servo
Copy link
Contributor

bors-servo commented May 2, 2016

Testing commit 22513a6 with merge 7528ecb...

bors-servo added a commit that referenced this pull request May 2, 2016
Support plain number-typed properties in geckolib

r? @bholley

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

bors-servo commented May 2, 2016

💔 Test failed - linux-dev

@heycam heycam force-pushed the heycam:number branch from 22513a6 to 53649b1 May 2, 2016
@heycam
Copy link
Member Author

heycam commented May 3, 2016

@bors-servo r=bholley

@bors-servo
Copy link
Contributor

bors-servo commented May 3, 2016

📌 Commit 53649b1 has been approved by bholley

@bors-servo
Copy link
Contributor

bors-servo commented May 3, 2016

Testing commit 53649b1 with merge ddada69...

bors-servo added a commit that referenced this pull request May 3, 2016
Support plain number-typed properties in geckolib

r? @bholley

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

bors-servo commented May 3, 2016

💔 Test failed - linux-rel

@highfive
Copy link

highfive commented May 3, 2016

  ▶ FAIL [expected PASS] /_mozilla/css/iframe/mozbrowser_navigation.html
  └   → /_mozilla/css/iframe/mozbrowser_navigation.html d9d9e489bc5b4508c01b06c7ff92f19dcc2ece3c
/_mozilla/css/iframe/mozbrowser_navigation_ref.html 8250706e506ad24231cc23be315b9b868387c598
Testing d9d9e489bc5b4508c01b06c7ff92f19dcc2ece3c == 8250706e506ad24231cc23be315b9b868387c598
@heycam
Copy link
Member Author

heycam commented May 3, 2016

@bors-servo
Copy link
Contributor

bors-servo commented May 3, 2016

Previous build results for android, arm32, arm64, linux-dev, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows are reusable. Rebuilding only linux-rel...

@KiChjang KiChjang assigned bholley and unassigned Ms2ger May 3, 2016
@bors-servo
Copy link
Contributor

bors-servo commented May 3, 2016

@bors-servo bors-servo merged commit 53649b1 into servo:master May 3, 2016
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
bors-servo added a commit that referenced this pull request May 4, 2016
Support color-typed properties in geckolib

This is on top of #10957 though I'm not sure how to make that PR's commits not appear in this one.

r? @bholley

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10959)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this pull request May 4, 2016
Support color-typed properties in geckolib

This is on top of #10957 though I'm not sure how to make that PR's commits not appear in this one.

r? @bholley

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10959)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this pull request May 4, 2016
Support color-typed properties in geckolib

This is on top of #10957 though I'm not sure how to make that PR's commits not appear in this one.

r? @bholley

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10959)
<!-- Reviewable:end -->
@heycam heycam deleted the heycam:number branch May 25, 2016
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 1, 2019
…m heycam:color); r=bholley

This is on top of servo/servo#10957 though I'm not sure how to make that PR's commits not appear in this one.

r? bholley

Source-Repo: https://github.com/servo/servo
Source-Revision: 1a1ea30f8c36a5dd01717fbf2294707f5d78c377

UltraBlame original commit: 36634609ea90b79d84ab07a959df7644a8f40d37
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 1, 2019
…m heycam:color); r=bholley

This is on top of servo/servo#10957 though I'm not sure how to make that PR's commits not appear in this one.

r? bholley

Source-Repo: https://github.com/servo/servo
Source-Revision: 1a1ea30f8c36a5dd01717fbf2294707f5d78c377

UltraBlame original commit: 36634609ea90b79d84ab07a959df7644a8f40d37
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 1, 2019
…m heycam:color); r=bholley

This is on top of servo/servo#10957 though I'm not sure how to make that PR's commits not appear in this one.

r? bholley

Source-Repo: https://github.com/servo/servo
Source-Revision: 1a1ea30f8c36a5dd01717fbf2294707f5d78c377

UltraBlame original commit: 36634609ea90b79d84ab07a959df7644a8f40d37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.