linstula / ember-cli-bootstrap

ember-cli addon for Twitter Bootstrap
37 stars 20 forks source link

Several bug fixes and improvements #37

Closed Panman82 closed 8 years ago

Panman82 commented 9 years ago

After working on ember-cli-bootswatch, I discovered several issues with the latest release of this addon. This PR fixes several bugs and adds some improvements:

I'd suggest at least fixing the shim.js error and incorrect option statements. Then release a new version since the current version of this addon is un-usable. Thanks for your consideration!

Panman82 commented 9 years ago

@linstula Thoughts on this PR? I made one change today but think this should be solid.

linstula commented 9 years ago

@Panman8201 I made some comments on the different commits. A few changes will need to be made but looks good. Thanks for your work, a lot of good additions.

Panman82 commented 9 years ago

@linstula I'll try to update this PR per your comments sometime this weekend. I seen that you fixed the import options issue so I'll remove that.

So you don't want to import the minified files? It'd be nice if ember-cli would know which files are minified to not re-minify, reducing overhead. I guess I've been using the minified files in my addons..

linstula commented 9 years ago

End users should have the ability to decide whether they want to minfy assets (for example, if you want to debug code in production but don't want to minify the assets so you can actually read the code). Ember-cli gives users a way to specify whether to minify or not. If we always use minified code when the environment is in production, we break the users ability to customize this option. So we should rely on the options passed to ember-cli and not preemptively use minified assets.

On Saturday, January 10, 2015, Ryan Panning notifications@github.com wrote:

@linstula https://github.com/linstula I'll try to update this PR per your comments sometime this weekend. I seen that you fixed the import options issue so I'll remove that.

So you don't want to import the minified files? It'd be nice if ember-cli would know which files are minified to not re-minify, reducing overhead. I guess I've been using the minified files in my addons..

— Reply to this email directly or view it on GitHub https://github.com/linstula/ember-cli-bootstrap/pull/37#issuecomment-69470065 .

Panman82 commented 8 years ago

Going through my forked repos ATM. This one is long out-of-date so I'll just close it.