support 'mixinsources' option - #131
Conversation
There was a problem hiding this comment.
I am proposing to move this filtering code out of "loadMixins" to keep "loadMixins" simpler and focused on doing one thing only.
7099aa7 to
4e15c52
Compare
There was a problem hiding this comment.
This may create duplicate items in the mixins array, when the same mixin is loaded from mixinDirs and mixinSources, is that correct? In which case you should de-duplicate the instructions.
4e15c52 to
7f11a15
Compare
|
@bajtos - Addressed your comments above and have also added a few more test cases. Can you please review? |
There was a problem hiding this comment.
I find this difficult to grasp at the first sight, it looks like loadMixins should be modifying the files variable.
|
The implementation looks good in general now, although I find it difficult to understand the implementation in compiler. Can you please try to simplify it a bit more, using my comments above for inspiration? Don't worry about it too much. If the result does not look better than the current solution or it takes too much time, then I am ok to land the patch as it is now. |
d460161 to
77c8c60
Compare
There was a problem hiding this comment.
nitpick: please rename to findMixinDefinitions (plural).
- By default looks for mixinsources in directory - Loads only mixins used through models
77c8c60 to
53f5182
Compare
|
Reworked on |
In continuation with - #79, added support for
mixinSourcesoption.The implementation -
mixinsdirectoryConnect to #79
@bajtos - Can you review?