-
Notifications
You must be signed in to change notification settings - Fork 16
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
Pull toastr from npm instead of bower #16
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this! Just a few comments, but everything else looks good.
index.js
Outdated
target.import( | ||
{ | ||
development: vendor + '/toastr/toastr.js', | ||
production: vendor + '/toastr/build/toastr.min.js' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be necessary since ember-cli does a production build itself.
index.js
Outdated
target.import( | ||
{ | ||
development: vendor + '/toastr/build/toastr.css', | ||
production: vendor + '/toastr/build/toastr.min.css' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
package.json
Outdated
@@ -43,7 +45,8 @@ | |||
"toastr" | |||
], | |||
"dependencies": { | |||
"ember-cli-babel": "^5.0.0" | |||
"ember-cli-babel": "^5.0.0", | |||
"toastr": "^2.1.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this gets added to the user's project, it shouldn't be here (only in dev deps for dummy app).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great thanks for the comments will do the changes :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for toastr, not for ember-cli-babel. That needs to stay for transpilation of the addon.
i just tried to install |
I just merged this and haven't published the changes yet. Will try to get to it later today. |
alright, sorry for being impatient ;) |
@knownasilya is there any way you could publish to npm please |
All set @gramozkrasniqi |
Resolve #14
Other changes:
Travis build file changed to test for node 4 and 6.
Remove testing for older nodes beacuse of ES6 language support.
Tests passing locally, travis is green also.