style: Custom properties cleanup. #18745
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This will give a nicer error message, plus it's more compact.
|
Heads up! This PR modifies the following files:
|
|
|
|
@bors-servo r+ |
|
📌 Commit 089d5ee has been approved by |
highfive
added
S-awaiting-merge
jdm
reviewed
Oct 4, 2017
| @@ -607,7 +607,7 @@ fn substitute_one( | |||
| if invalid.contains(name) { | |||
| return Err(()); | |||
| } | |||
| let computed_value = if specified_value.references.map(|set| set.is_empty()) == Some(false) { | |||
| let computed_value = if specified_value.references.map_or(false, |set| !set.is_empty()) { | |||
There was a problem hiding this comment.
This is not equivalent to the previous behaviour. Is that intentional?
|
@bors-servo: r- until my question is answered. |
jdm
added
S-awaiting-review
|
@bors-servo: r=nox |
|
📌 Commit 089d5ee has been approved by |
highfive
added
S-awaiting-merge
|
@bors-servo p=1 |
bors-servo
pushed a commit
that referenced
this pull request
Oct 5, 2017
style: Custom properties cleanup. Use less unwrap and custom types in custom properties. The idea is for this to shed some light in https://bugzilla.mozilla.org/show_bug.cgi?id=1403845. <!-- 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/18745) <!-- Reviewable:end -->
|
💔 Test failed - mac-rel-wpt1 |
highfive
added
S-tests-failed
|
@bors-servo retry |
bors-servo
pushed a commit
that referenced
this pull request
Oct 5, 2017
style: Custom properties cleanup. Use less unwrap and custom types in custom properties. The idea is for this to shed some light in https://bugzilla.mozilla.org/show_bug.cgi?id=1403845. <!-- 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/18745) <!-- Reviewable:end -->
highfive
added
S-awaiting-merge
|
☀️ 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 |
highfive
removed
the
S-awaiting-merge
emilio
added a commit
to emilio/servo
that referenced
this pull request
Oct 5, 2017
In servo#18745, I replaced a few manual iterations over `index` with the iterator, and it changed the behavior of `layout/style/test/test_variables_order.html`, since it turns out that the iterator iterates right to left. I think this is just an accident that happened due to inconsistencies in how we were iterating over it, and that our behavior was inconsistent (since we iterated rtl in some cases, but ltr in others seems like it'd be inconsistent depending on the depth of the tree and different stuff). This brings back the expected behavior again, and ensures we iterate over a consistent order every time.
emilio
added a commit
to emilio/servo
that referenced
this pull request
Oct 5, 2017
In servo#18745, I replaced a few manual iterations over `index` with the iterator, and it changed the behavior of `layout/style/test/test_variables_order.html`, since it turns out that the iterator iterates right to left. I think this is just an accident that happened due to inconsistencies in how we were iterating over it, and that our behavior was inconsistent (since we iterated rtl in some cases, but ltr in others seems like it'd be inconsistent depending on the depth of the tree and different stuff). This brings back the expected behavior again, and ensures we iterate over a consistent order every time.
bors-servo
pushed a commit
that referenced
this pull request
Oct 5, 2017
style: Iterate in the expected order in the custom_properties module. In #18745, I replaced a few manual iterations over `index` with the iterator, and it changed the behavior of `layout/style/test/test_variables_order.html`, since it turns out that the iterator iterates right to left. I think this is just an accident that happened due to inconsistencies in how we were iterating over it, and that our behavior was inconsistent (since we iterated rtl in some cases, but ltr in others seems like it'd be inconsistent depending on the depth of the tree and different stuff). This brings back the expected behavior again, and ensures we iterate over a consistent order every time. <!-- 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/18761) <!-- Reviewable:end -->
avadacatavra
pushed a commit
to avadacatavra/servo
that referenced
this pull request
Oct 17, 2017
In servo#18745, I replaced a few manual iterations over `index` with the iterator, and it changed the behavior of `layout/style/test/test_variables_order.html`, since it turns out that the iterator iterates right to left. I think this is just an accident that happened due to inconsistencies in how we were iterating over it, and that our behavior was inconsistent (since we iterated rtl in some cases, but ltr in others seems like it'd be inconsistent depending on the depth of the tree and different stuff). This brings back the expected behavior again, and ensures we iterate over a consistent order every time.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use less unwrap and custom types in custom properties.
The idea is for this to shed some light in https://bugzilla.mozilla.org/show_bug.cgi?id=1403845.
This change is