-
Notifications
You must be signed in to change notification settings - Fork 141
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
Update ember cli and convert to DDAU #123
Conversation
98cb024
to
a64eaeb
Compare
Testem default window size increased which caused failures when 500px no longer was far enough
a64eaeb
to
b3f5c8f
Compare
Thanks for this! Wasn't expecting this to come in. Obviously this will be a major breaking change for existing users. I'll need to mark it as such. I'll need to dig in here and explore this PR in depth first before I can pull it in. |
Let me know if you have any questions. I tried to just forklift the addon with no changes other than moving to closure actions. Happy to help with whatever it needs to get this merged. |
This would be really helpful - I was about to look into filing an issue to upgrade ember-cli-babel because of a deprecation warning coming from ember-cli-version-checker, but this PR fixes that too. |
Really looking forward to this as well! |
@dgavey - is there any progress on this? |
Unfortunately no, I've taken a new job, and I really haven't found any extra time for working on this project currently. If someone want's to take it up to become a maintainer, I would be happy to add them. |
@stopdropandrew - perhaps you could create separate PRs (one for the first and second commits, one for the third commit and one for the DDAU stuff) which could be reviewed and merged independently (by @dgavey and the rest of us). The first two commits are the most urgent ones right now because of the deprecation warnings. The rest could follow later and with a major version bump because of the breaking changes. Does that sound reasonable? |
It's been awhile since working on this, but as I recall I took that larger leap because there were new deprecation warnings introduced when upgrading ember-cli. But we can certainly split them up. |
@dgavey - could you take another look at this PR please? I've been using it in production for a couple of months now and seems stable. @stopdropandrew - do you mind rebasing on master so the branch can be merged? And, if @dgavey requests so, perhaps split it into a couple of new PRs? |
@dgavey the breaking changes come from the DDAU changes? What about checking if the action attribute is a string or a function to decide how to call/trigger the action? |
@mharris717, @stopdropandrew, @dgavey - Ember 3.4 will deprecate The breaking change can be "fixed" by incrementing the major version of the library. It's there for exactly that. :) |
I'll try to find time this week to at least get the addon upgraded to the latest Ember on a separate branch. |
Yes I will use this branch and create a new release off of it soon, sorry about the delay on this. |
Ok Merged, thanks for your patience. You should be able to use this with 0.5.0 I'm going to get it updated to the latest ember with another version bump hopefully shortly. |
Does anyone in this thread happen to know when there are two more instances of |
@jackiehluo: If I read the code, the sendAction lines are still there to provide support for Ember < 1.13. @Team: Since Ember 3.4 deprecates the |
This pull upgrades to the latest Ember (fixes #121), and in the process switches to DDAU (fixes #120 and fixes #59).