Remove type Impl from traits PseudoElement and NonTSPseudoClass - #432
Remove type Impl from traits PseudoElement and NonTSPseudoClass#432dislogical wants to merge 2 commits into
type Impl from traits PseudoElement and NonTSPseudoClass#432Conversation
|
Are these used in Gecko? If so, I'm not sure we should remove them. We are downstream of Gecko, so this sort of change should likely happen there. |
|
Great question! I’m new here and can’t find any obvious docs on how to check, is there anything I should be looking at? |
These aren't actually used anywhere, and make re-using Pseudo implementations between different parsers MUCH more complicated. This is technically a breaking change, as downstreams will need to remove the `type` definition, but that's all that's required (see style/servo/selector_parser.rs for required changes).
1fa7faa to
8e876e7
Compare
|
Try applying your patch to https://github.com/mozilla-firefox/firefox (stylo is in the servo directory) and see if it builds. |
|
Ok just confirmed that this patch does compile in upstream Firefox. Does that mean I should submit this patch there? |
|
@dislogical That would be ideal as then we wouldn't have to upstream it and would just incorporate it the next time we sync stylo. |
|
@mrobinson sounds good, I'll give it a shot! I know their barrier to contribution entry is a bit higher than "PRs welcome," so I'm hoping they'll take such a small change. |
|
Upstream review: https://phabricator.services.mozilla.com/D316430 |
|
@dislogical Thank you so much! I'll close this in favor of the upstream contribution. |
|
No problem! It was easier than I thought. How often do y'all pull updates from upstream? |
|
Every month |
4 participants
These aren't actually used anywhere, and make re-using Pseudo implementations between different parsers MUCH more complicated.
This is technically a breaking change, as downstreams will need to remove the
typedefinition, but that's all that's required (see style/servo/selector_parser.rs for required changes).