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.

Sign up for GitHub

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

Fix counter() and counters() serialization. #20224

Merged
merged 1 commit into from Mar 14, 2018
Merged

Fix counter() and counters() serialization. #20224

merged 1 commit into from Mar 14, 2018

Conversation

emilio
Copy link
Member

@emilio emilio commented Mar 7, 2018 •
edited by SimonSapin

@highfive
Copy link

highfive commented Mar 7, 2018

Heads up! This PR modifies the following files:

  • @bholley: components/style/properties/longhand/list.mako.rs, components/style/values/specified/counters.rs, components/style/values/generics/mod.rs
  • @canaltinova: components/style/properties/longhand/list.mako.rs, components/style/values/specified/counters.rs, components/style/values/generics/mod.rs

@highfive
Copy link

highfive commented Mar 7, 2018

warning Warning warning

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

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 7, 2018
@emilio
Copy link
Member Author

emilio commented Mar 7, 2018

r? @nox

@highfive highfive assigned nox and unassigned mbrubeck Mar 7, 2018
@emilio
Copy link
Member Author

emilio commented Mar 7, 2018

(And / or @upsuper)

@emilio emilio mentioned this pull request Mar 7, 2018
Merged
@emilio
Copy link
Member Author

emilio commented Mar 7, 2018

Don't merge yet since I probably need to update tests or what not.

@emilio
Copy link
Member Author

emilio commented Mar 7, 2018

Oh, actually in the computed value gecko already serializes it properly.

@upsuper
Copy link
Contributor

upsuper commented Mar 7, 2018

Reviewed 1 of 1 files at r1, 1 of 1 files at r2, 1 of 1 files at r3, 2 of 2 files at r4, 1 of 1 files at r5.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

Comments from Reviewable

@upsuper
Copy link
Contributor

upsuper commented Mar 7, 2018

LGTM, but I'd leave the derive part to @nox. I'm not confident enough to review that.

@nox
Copy link
Contributor

nox commented Mar 7, 2018

r=me, I suspect there are other potential uses of #[css(skip_if)] but I can find those myself.

@nox
Copy link
Contributor

nox commented Mar 7, 2018

LGTM, land it whenever you want but my own PR is probably going to bitrot that. :p

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #20230) made this pull request unmergeable. Please resolve the merge conflicts.

@highfive highfive added the S-needs-rebase There are merge conflict errors. label Mar 7, 2018
@emilio
Copy link
Member Author

emilio commented Mar 7, 2018

@bors-servo r=nox

@bors-servo
Copy link
Contributor

📌 Commit 8e56200 has been approved by nox

@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. S-needs-rebase There are merge conflict errors. labels Mar 7, 2018
@bors-servo
Copy link
Contributor

⌛ Testing commit 8e56200 with merge 8be6991...

bors-servo pushed a commit that referenced this pull request Mar 7, 2018
Auto merge of #20224 - emilio:counter-serialization, r=nox
8be6991 
Fix counter() and counters() serialization.

See w3c/csswg-drafts#670 and web-platform-tests/wpt#9862.

<!-- 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/20224)
<!-- 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 Mar 7, 2018
@emilio emilio mentioned this pull request Mar 8, 2018
Merged
@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Mar 14, 2018
@emilio
Copy link
Member Author

emilio commented Mar 14, 2018

@bors-servo r=nox

@bors-servo
Copy link
Contributor

📌 Commit 839eda0 has been approved by nox

@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 Mar 14, 2018
@bors-servo
Copy link
Contributor

⌛ Testing commit 839eda0 with merge c4887c5...

bors-servo pushed a commit that referenced this pull request Mar 14, 2018
Auto merge of #20224 - emilio:counter-serialization, r=nox
c4887c5 
Fix counter() and counters() serialization.

See w3c/csswg-drafts#670 and web-platform-tests/wpt#9862.

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

💔 Test failed - mac-rel-wpt2

@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 Mar 14, 2018
@emilio
Copy link
Member Author

emilio commented Mar 14, 2018

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 839eda0 with merge 7b32652...

bors-servo pushed a commit that referenced this pull request Mar 14, 2018
Auto merge of #20224 - emilio:counter-serialization, r=nox
7b32652 
Fix counter() and counters() serialization.

See w3c/csswg-drafts#670 and web-platform-tests/wpt#9862.

<!-- 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/20224)
<!-- Reviewable:end -->
@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 Mar 14, 2018
@bors-servo
Copy link
Contributor

💔 Test failed - mac-rel-css2

@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 Mar 14, 2018
@jdm
Copy link
Member

jdm commented Mar 14, 2018

@bors-servo retry

@bors-servo
Copy link
Contributor

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

@bors-servo bors-servo mentioned this pull request Mar 14, 2018
Merged
5 tasks
@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: nox
Pushing 7b32652 to master...

@bors-servo bors-servo merged commit 839eda0 into servo:master Mar 14, 2018
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Mar 15, 2018
@emilio
No bug - update WPT expectations for servo/servo#20224. r=me
dd9fa5e 
This is a test change that hasn't been synced yet:

  web-platform-tests/wpt#9862

Once it is it'll start passing.

MozReview-Commit-ID: H0RZlep7oX3
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 2, 2019
@marco-c
No bug - update WPT expectations for servo/servo#20224. r=me
af2c65b 
This is a test change that hasn't been synced yet:

  web-platform-tests/wpt#9862

Once it is it'll start passing.

MozReview-Commit-ID: H0RZlep7oX3

UltraBlame original commit: 31b8b62832d505d62f66d56f90cfc4b1c7e8a557
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 2, 2019
@marco-c
No bug - update WPT expectations for servo/servo#20224. r=me
abb21ae 
This is a test change that hasn't been synced yet:

  web-platform-tests/wpt#9862

Once it is it'll start passing.

MozReview-Commit-ID: H0RZlep7oX3

UltraBlame original commit: 31b8b62832d505d62f66d56f90cfc4b1c7e8a557
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 2, 2019
@marco-c
No bug - update WPT expectations for servo/servo#20224. r=me
9e1f10d 
This is a test change that hasn't been synced yet:

  web-platform-tests/wpt#9862

Once it is it'll start passing.

MozReview-Commit-ID: H0RZlep7oX3

UltraBlame original commit: 31b8b62832d505d62f66d56f90cfc4b1c7e8a557
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nox nox

Labels
S-tests-failed The changes caused existing tests to fail.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@emilio @highfive @upsuper @nox @bors-servo @jdm @mbrubeck