Skip to content
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

Add SASS support to project #880

Merged
merged 4 commits into from
Mar 31, 2017
Merged

Add SASS support to project #880

merged 4 commits into from
Mar 31, 2017

Conversation

pmizio
Copy link
Contributor

@pmizio pmizio commented Mar 31, 2017

No description provided.

@amilajack
Copy link
Member

amilajack commented Mar 31, 2017

I love how this PR adds support for sass out of the box. But I dont want to make knowing sass a barrier to using the boilerplate. How about we keep keep the .css files as they were but keep all the other changes this PR brings?

Also thanks for this PR! 😄

@amilajack amilajack self-requested a review March 31, 2017 16:43
@pmizio
Copy link
Contributor Author

pmizio commented Mar 31, 2017

Ok so i revert to original .css files and keep my SASS configs as option to use.

@amilajack
Copy link
Member

Yea. also I would recommend commenting why the sass webpack configs are there. Someone may think they are unnecessary in the future and try to remove them. Comment something like 'these are hear for out of the box support for sass'

@amilajack
Copy link
Member

Also I forgot to ask. Does this work with live reloading? What's the performance like?

@pmizio
Copy link
Contributor Author

pmizio commented Mar 31, 2017

Ok, I reverted all css files to state from master branch and short feature description in README.md.
I also added some feature comments in webpack config files.

@amilajack
Copy link
Member

Does this work with hot reloading?

@pmizio
Copy link
Contributor Author

pmizio commented Mar 31, 2017

Yes it is, reloads view almost instantly so I think it performs well.

@amilajack
Copy link
Member

amilajack commented Mar 31, 2017

Great! Also is it possible to add the webpack configs to the webpack base config? What was the reason for adding to both dev and prod configs?

Edit: Never mind. The config makes sense now! Merging 🎉

@amilajack amilajack merged commit 265ef0a into electron-react-boilerplate:master Mar 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants