search

shakacode / bootstrap-sass-loader

Webpack Loader for the Sass version Twitter Bootstrap
MIT License
118 stars 25 forks source link

Made mainSass config item optional #6

Closed azamat-sharapov closed 9 years ago

azamat-sharapov commented 9 years ago

5

justin808 commented 9 years ago
  1. Did you test both cases?
  2. Update readme
  3. one suggestion
azamat-sharapov commented 9 years ago
  1. Sure
  2. Updated
  3. ?

Also added space after if

justin808 commented 9 years ago

@azamat-sharapov I'm going to have this merged by this evening. I'm tuning up my example as well. I'm considering removing support for having no config file, to simplify the project.

Any need for the no-config option?

It seems that if you're not going to customize the bootstrap installation, then there's not a huge advantage in using the sass version. I.e., you might as well use the built CSS files.

Thoughts?

justin808 commented 9 years ago

@azamat-sharapov Please see

https://github.com/justin808/bootstrap-sass-loader-example/pull/2

Could you please try out your changes in the context of this branch. Thanks collaborating.

If you run webpack in that project, you can put in print statements in the loader (assuming you used npm link. You can also run node-debug webpack and put in a debugger statement.

azamat-sharapov commented 9 years ago

Any need for the no-config option?

@justin808 absolutely! I'm using your loader mainly because I can customize bootstrap.

Could you please try out your changes in the context of this branch. Thanks collaborating.

Sorry, our timezone seems to be very different, so I just got your messages. I see you already have fixed that, but I'll also try on my end and let you know, if I notice any problem.

justin808 commented 9 years ago

I actually fixed things. Options for config or no-config, and config even allows the extract-text plugin.

azamat-sharapov commented 9 years ago

yes, I noticed that later, later :) Optional config files is the way to go, thanks.