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

Use macro to impl From for font variant #19282

Merged
merged 1 commit into from Nov 20, 2017
Merged

Use macro to impl From for font variant #19282

merged 1 commit into from Nov 20, 2017

Conversation

CYBAI
Copy link
Member

@CYBAI CYBAI commented Nov 19, 2017 •
edited by SimonSapin

As I discussed in #19277, I'd like to change the implementation for From of font variant to use macro, impl_gecko_keyword_conversions.
r? @emilio

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes update From impl for font variant to use macro impl_gecko_keyword_conversions.
  • These changes do not require tests

This change is Reviewable

@highfive
Copy link

Heads up! This PR modifies the following files:

  • @bholley: components/style/values/specified/font.rs
  • @canaltinova: components/style/values/specified/font.rs
  • @emilio: components/style/values/specified/font.rs

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 19, 2017
@highfive
Copy link

warning Warning warning

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

canova
canova approved these changes Nov 19, 2017
View reviewed changes
Copy link
Contributor

@canova canova left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks!

@canova
Copy link
Contributor

canova commented Nov 19, 2017

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 3e9d493 has been approved by canaltinova

@highfive highfive assigned canova and unassigned emilio Nov 19, 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 Nov 19, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit 3e9d493 with merge 6903a66...

bors-servo pushed a commit that referenced this pull request Nov 19, 2017
Auto merge of #19282 - CYBAI:use-macro-for-font-variant, r=canaltinova
6903a66 
Use macro to impl From for font variant

As I discussed in #19277, I'd like to change the implementation for `From` of font variant to use macro, `impl_gecko_keyword_conversions`.
r? @emilio

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes update `From` impl for font variant to use macro `impl_gecko_keyword_conversions`.
- [x] These changes do not require tests

<!-- 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/19282)
<!-- 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 Nov 19, 2017
@canova
Copy link
Contributor

canova commented Nov 19, 2017 •
edited by KiChjang

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 3e9d493 with merge 9afc057...

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Nov 19, 2017
bors-servo pushed a commit that referenced this pull request Nov 19, 2017
Auto merge of #19282 - CYBAI:use-macro-for-font-variant, r=canaltinova
9afc057 
Use macro to impl From for font variant

As I discussed in #19277, I'd like to change the implementation for `From` of font variant to use macro, `impl_gecko_keyword_conversions`.
r? @emilio

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes update `From` impl for font variant to use macro `impl_gecko_keyword_conversions`.
- [x] These changes do not require tests

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

💔 Test failed - linux-rel-css

@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 Nov 19, 2017
@CYBAI
Copy link
Member Author

CYBAI commented Nov 20, 2017

Hmm... it seems that it failed with intermittent again

@KiChjang
Copy link
Contributor

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 3e9d493 with merge 63bd783...

bors-servo pushed a commit that referenced this pull request Nov 20, 2017
Auto merge of #19282 - CYBAI:use-macro-for-font-variant, r=canaltinova
63bd783 
Use macro to impl From for font variant

As I discussed in #19277, I'd like to change the implementation for `From` of font variant to use macro, `impl_gecko_keyword_conversions`.
r? @emilio

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes update `From` impl for font variant to use macro `impl_gecko_keyword_conversions`.
- [x] These changes do not require tests

<!-- 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/19282)
<!-- Reviewable:end -->
@highfive highfive removed the S-tests-failed The changes caused existing tests to fail. label Nov 20, 2017
@highfive highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Nov 20, 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: canaltinova
Pushing 63bd783 to master...

@bors-servo bors-servo merged commit 3e9d493 into servo:master Nov 20, 2017
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Nov 20, 2017
@CYBAI CYBAI deleted the use-macro-for-font-variant branch November 20, 2017 05:05
@CYBAI
Copy link
Member Author

CYBAI commented Nov 20, 2017 •
edited

@KiChjang Phew! It passed! Thanks for the retry!
Also, thanks @canaltinova for review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@canova canova canova approved these changes

Assignees

@canova canova

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@CYBAI @highfive @canova @bors-servo @KiChjang @emilio