search

Tonkpils / ember-sweetalert

Ember CLI addon for SweetAlert2
MIT License
19 stars 10 forks source link

Updated To Sweetalert2 version 7.19.0 #12

Closed Shaneprrlt closed 6 years ago

Tonkpils commented 6 years ago

Thanks for taking the time to submit this PR @Shaneprrlt

This all looks good to me. @lindyhopchris would you be able to test this to make sure it works correctly?

Since this is part of https://github.com/Tonkpils/ember-sweetalert/issues/10 I think it'd get us a step closer to that 😄

lindyhopchris commented 6 years ago

I get errors with an npm install, which is something to do with this commit adding a package-lock.json. If I delete the lock file and re-run npm install it runs fine. I suggest we don't add the package-lock on this PR?

The other thing is the ember test fails on this PR but doesn't on master. I think this might be related to us needing to update to a newer version of Ember. I'll see if I can do that quickly now and push it to a new branch.

One other thing about this PR - it would need to be released as 0.2.0 rather than 0.1.2 because changing to a different major version of sweetalert could break consuming applications.

lindyhopchris commented 6 years ago

Have pushed a develop branch that has the addon on Ember 2.18. Wanted to do that jump first rather than going straight to 3.1. Might be worth doing this PR against the develop branch?

Tonkpils commented 6 years ago

That sounds good to me, @Shaneprrlt could you remove the addition of package-lock and remove the change to the version bump on package.json as we would take care of bumping the version once a new release is ready to go out.

Once that's done, I'll point this to develop so we can merge that there

Shaneprrlt commented 6 years ago

@Tonkpils Yep, I'll get that fix pushed.

Shaneprrlt commented 6 years ago

@Tonkpils Apologies for the delay, just pushed those changes though.