-
Notifications
You must be signed in to change notification settings - Fork 2k
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 for mixins declaration with space before colon #2322
Conversation
Patches included: * Polymer/polymer#2205 * Polymer/polymer#2247 * Polymer/polymer#2258 * Polymer/polymer#2266 * Polymer/polymer#2286 (accepted upstream) * Polymer/polymer#2295 * Polymer/polymer#2322
60cb3bc
to
d3bda2b
Compare
Thanks for the pull request - apologies for the delay here. This looks like a solid fix - would you mind squashing the commits? |
0d31daf
to
e91048c
Compare
Rebased against master and squashed into single commit. |
👍 lgtm |
Allow any space character or even `{` and `}` (before and after capturing pattern correspondingly) as pattern boundaries instead of new lines only. In minified sources there might be no space, semicolon or line start, so we need to account that as well.
e91048c
to
883aa5c
Compare
Patches applied on top of stable version: * Polymer/polymer#2205 * Polymer/polymer#2247 * Polymer/polymer#2295 * Polymer/polymer#2322 * Polymer/polymer#2349 * Polymer/polymer#2419
I think this should be good to go, @tjsavage anything else here? |
LGTM - @azakus any idea why the tests would be failing? |
It doesn't look like these tests have been run since our fix in webcomponentsjs for the failing IE tests. |
How to trigger it? |
The IE failures are fixed with 1.2.0, this PR should pass after merging. |
@nazar-pc already did. This PR was using the old homegrown CI, new PRs will use Travis CI. |
Confirm IE 10 and IE 11 passing after merging with Polymer on master. |
LGTM |
Fix for mixins declaration with space before colon
With this fix both work fine.