Skip to content

Conversation

@cybercase
Copy link
Contributor

Adds support for multiple webpack config files. See issue #14

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not something like,

user_config = [dict(DEFAULT_CONFIG, **config) for config in user_config.values()] 

I'm not sure if we should raise a custom error here. It should just fail normally. We should add a django system check to make sure new config is being used. Check https://docs.djangoproject.com/en/1.8/ref/checks/#api-reference

We might have to add apps.py and hook into it's ready method. This will only work on 1.7 and above. It'll fail silently on 1.6. I think that is OK.

@owais
Copy link
Collaborator

owais commented Sep 6, 2015

Stefano, thank you so much for this! I'll go over it today and test it locally. Looks like it should mostly work. Once it is ready for merging, I'll have to ask you for 2 things. First is to rebase -i on master to squash all your commits into a single feature commit (you can use any method to squash) and second is to see if you can figure out which new lines the test cases don't cover any more and cover them. (coerage has fallen to 94% from 100%).

Again, thanks a lot for this. I think a lot of people will benefit from this PR.

@cybercase
Copy link
Contributor Author

Thanks for your review.
I'm happy to contribute to this project! I think it's a great way to deal with webpack from django.

I've squashed commits together and restored the 100% coverage for django 1.8 and 1.7.
Unfortunately I couldn't get the same coverage for django 1.6 due to the missing System check framework...

@cybercase
Copy link
Contributor Author

Hello @owais. Any news for this PR?

@owais
Copy link
Collaborator

owais commented Sep 9, 2015

@cybercase I'm buried right now. I'll merge this and release a new version before EOW.

@owais owais merged commit 784f7b8 into django-webpack:master Sep 10, 2015
@owais
Copy link
Collaborator

owais commented Sep 10, 2015

Thanks for this Stefano! I made some minor changes to a few things you did after the merge. Hope you don't mind :)

@cybercase
Copy link
Contributor Author

Not at all... great changes!
Thanks for merging

@littlehome-eugene
Copy link

great.. didn't know this feature existed.. good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants