-
Notifications
You must be signed in to change notification settings - Fork 213
Conversation
WIll fix tests momentarily |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Tests are passing now! |
@eliperelman if you have a chance for another review, take a look. I made some tweaks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for working on this - these should be the last few changes I think :-)
Could you also add an entry to the migration guide? It can probably be quite vague and just mention switching to use oneOf and some of the configuration options may have changed etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for those changes - PR looks great now!
Could you add the migration guide entry and then I think we're all set to merge? :-)
Forgot! Now added 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! 😺
Applies Vue style rules as oneOfs, following #1281. * Used https://github.com/vuejs/vue-cli/blob/dev/packages/%40vue/cli-service/lib/config/css.js#L72-L168 as a model * Fixes #1243
#1243 (comment)
Following this merge, we can rebase #1276 and merge that in.