Skip to content

/ servo

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

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 commented Mar 7, 2018 •
edited by SimonSapin

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

This change is Reviewable
@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!
@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
Member

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
Member

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
Member

nox commented Mar 7, 2018

r=me, I suspect there are other potential uses of #[css(skip_if)] but I can find those myself.
@emilio emilio force-pushed the emilio:counter-serialization branch from b476b04 to ef422dd Mar 7, 2018
@nox
Copy link
Member

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

bors-servo commented Mar 7, 2018

☔ The latest upstream changes (presumably #20230) made this pull request unmergeable. Please resolve the merge conflicts.
@emilio emilio force-pushed the emilio:counter-serialization branch from ef422dd to 8e56200 Mar 7, 2018
@emilio
Copy link
Member Author

emilio commented Mar 7, 2018

@bors-servo r=nox
@bors-servo
Copy link
Contributor

bors-servo commented Mar 7, 2018

📌 Commit 8e56200 has been approved by nox
@bors-servo
Copy link
Contributor

bors-servo commented Mar 7, 2018

⌛ Testing commit 8e56200 with merge 8be6991...
bors-servo added a commit that referenced this pull request Mar 7, 2018
@bors-servo
Auto merge of #20224 - emilio:counter-serialization, r=nox
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
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

bors-servo commented Mar 7, 2018

💔 Test failed - linux-rel-wpt
@emilio emilio mentioned this pull request Mar 8, 2018
Merged
@emilio
style: Avoid serializing the counter type in counter functions if it'…
Unverified
This user has not uploaded their public key yet.
GPG key ID: 056B727BB9C1027C Learn about signing commits
Loading status checks…
839eda0 
…s decimal.

See web-platform-tests/wpt#9862 and such.
@emilio emilio force-pushed the emilio:counter-serialization branch from 81e618f to 839eda0 Mar 14, 2018
@emilio
Copy link
Member Author

emilio commented Mar 14, 2018

@bors-servo r=nox
@bors-servo
Copy link
Contributor

bors-servo commented Mar 14, 2018

📌 Commit 839eda0 has been approved by nox
@bors-servo
Copy link
Contributor

bors-servo commented Mar 14, 2018

⌛ Testing commit 839eda0 with merge c4887c5...
bors-servo added a commit that referenced this pull request Mar 14, 2018
@bors-servo
Auto merge of #20224 - emilio:counter-serialization, r=nox
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
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

bors-servo commented Mar 14, 2018

💔 Test failed - mac-rel-wpt2
@emilio
Copy link
Member Author

emilio commented Mar 14, 2018

@bors-servo retry
@bors-servo
Copy link
Contributor

bors-servo commented Mar 14, 2018

⌛ Testing commit 839eda0 with merge 7b32652...
bors-servo added a commit that referenced this pull request Mar 14, 2018
@bors-servo
Auto merge of #20224 - emilio:counter-serialization, r=nox
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
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 -->
@bors-servo
Copy link
Contributor

bors-servo commented Mar 14, 2018

💔 Test failed - mac-rel-css2
@jdm
Copy link
Member

jdm commented Mar 14, 2018

@bors-servo retry
@bors-servo
Copy link
Contributor

bors-servo commented Mar 14, 2018

⚡ 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
2 of 5 tasks complete
@bors-servo
Copy link
Contributor

bors-servo commented Mar 14, 2018

☀️ 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
3 of 4 checks passed
3 of 4 checks passed
Taskcluster (pull_request) TaskGroup: failure
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
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
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
@emilio @highfive @upsuper @nox @bors-servo @jdm @mbrubeck
You can’t perform that action at this time.