Integrate npm bootstrap-sass; Remove bootstrap-sass gem; Update README#3
Integrate npm bootstrap-sass; Remove bootstrap-sass gem; Update README#3justin808 merged 20 commits intoshakacode:masterfrom
Conversation
There was a problem hiding this comment.
We should mention https://github.com/justin808/react-webpack-rails-tutorial/blob/master/Procfile.dev
foreman start -f Procfile.dev
|
@mbreining Anything pending? or ready to merge? |
|
@justin808, take another look and see if you want to merge. Note that I have not addressed your offline comment on ExtractTextPlugin (add the webpack/assets directory to the asset pipeline config). We should talk. |
|
I've changed my opinion on this based on a real project. Basically, my thought right now is:
There's a bug with the bootstrap-sass-loader in that it uses a full local path for the font directory. That's clearly a problem when publishing the CSS file for deployment. Thus, I'd rather let the bootstrap-sass gem and the rails asset pipeline handle all those thorny issues of paths for fonts and images, as well as fingerprinting. Throughts? |
|
Here's the issue on the path being absolute: shakacode/bootstrap-sass-loader#3 |
…nfigure asset pipeline's search patch to include webpack/assets/stylesheets
|
@justin808 please take another look. We can continue to iterate over time but this batch of changes should be ready to merge, pending your comments. |
There was a problem hiding this comment.
we will change this shortly, but this can be the next PR
There was a problem hiding this comment.
I think we should remove the comments on the extract text plugin.
I think for rails deployments, it's just not worth any complexity to have webpack extract the files...
The reason that we'd want webpack to do it is b/c you want the file for the webpack dev server testing...For right now, I'm not convinced.
|
Looking great! getting close! |
|
Ok, I think all comments are addressed. Please review. |
Integrate npm bootstrap-sass; Remove bootstrap-sass gem; Update README
Justin, please review. Let's chat tomorrow.