Skip to content
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

[WIP] Update core object #3056

Closed
wants to merge 22 commits into from
Closed

Conversation

jgwhite
Copy link
Contributor

@jgwhite jgwhite commented Jan 21, 2015

Current status: still testing against addons.

@@ -9,7 +9,6 @@ var SilentError = require('../errors/silent');
var reexport = require('../utilities/reexport');
var escapeRegExp= require('../utilities/escape-regexp');


var p = require('../preprocessors');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we rename this to preprocessors?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally.

@stefanpenner stefanpenner added this to the v0.2.0 milestone Jan 22, 2015
@krisselden
Copy link
Contributor

@jgwhite after working with the old core object, I'm a big fan of this.

@jgwhite jgwhite force-pushed the update-core-object branch 2 times, most recently from f967bb9 to bd8998c Compare January 24, 2015 21:52
@jgwhite
Copy link
Contributor Author

jgwhite commented Jan 24, 2015

This now passes but the changes are extensive. 😬

Making the switch without breaking our existing crop of addons involves two awkward contortions:

Other concerns:

  • Omitting the this._super.apply(this, arguments) in init will almost always break things
  • The difference between CoreObject#init and Ember.CoreObject#init may cause confusion

@stefanpenner
Copy link
Contributor

Omitting the this._super.apply(this, arguments) in init will almost always break things

we can fn.toString().indexOf('._super') and log a depreciation

Project detects and uses the appropriate constructor signature for addons

this should likely raise a deprecation (as i believe this will be a fail without)

@trabus
Copy link
Contributor

trabus commented Jan 26, 2015

Wow, awesome work! 👏

@jgwhite jgwhite force-pushed the update-core-object branch 7 times, most recently from b5176df to 21dfa02 Compare February 7, 2015 18:56
@jgwhite jgwhite force-pushed the update-core-object branch 2 times, most recently from 9ad41c5 to f235b1e Compare February 11, 2015 16:42
@jgwhite jgwhite force-pushed the update-core-object branch 2 times, most recently from 8030fe9 to 6b0e3b5 Compare February 22, 2015 15:13
@stefanpenner
Copy link
Contributor

@jgwhite im nervous that this is getting way out of sync. Ping me, I would like to be sure to unblock you, or take the reigns before we diverge entirely, I would love to get this in asap.

@jgwhite
Copy link
Contributor Author

jgwhite commented Feb 24, 2015

@stefanpenner I’ve been rebasing fairly regularly and I believe this is in a mergeable state right now.

@jgwhite jgwhite changed the title [WIP] Update core object Update core object Feb 24, 2015
@jgwhite
Copy link
Contributor Author

jgwhite commented Feb 24, 2015

Oops. Spoke too soon!

@jgwhite jgwhite changed the title Update core object [WIP] Update core object Feb 24, 2015
@stefanpenner
Copy link
Contributor

@jgwhite we should sync up, ping me when you are free. Would love to unblock you :)

@stefanpenner stefanpenner mentioned this pull request Mar 27, 2015
7 tasks
@jgwhite
Copy link
Contributor Author

jgwhite commented May 20, 2015

@stefanpenner I’ve been trying to find a way to roll out the new CoreObject without changing exsting libraries but, the more I look at it, the more I think this will need a symlink-style unilateral upgrade. It’s mainly variations in expectations around init that cause trouble.

broccoli-caching-writer is a particularly off-beat culprit. What do you think of releasing a new-core-object-friendly version: ember-cli/broccoli-caching-writer@ember-cli:master...jgwhite:upgrade-core-object

@stefanpenner
Copy link
Contributor

@jgwhite im not quite sure what that external lib was causing grief. But it does not need to change (and also no longer uses core object itself at all)

@stefanpenner
Copy link
Contributor

@jgwhite do you have interest in landing this, or should we close and revist later?

@jgwhite
Copy link
Contributor Author

jgwhite commented Nov 20, 2015

@stefanpenner much as it pains me to say it — let’s close and revisit later.

@stefanpenner
Copy link
Contributor

@jgwhite NP, future work :)

@btecu btecu mentioned this pull request Mar 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants