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

provide ember-core from NPM #13022

Merged
merged 12 commits into from
Oct 7, 2016
Merged

provide ember-core from NPM #13022

merged 12 commits into from
Oct 7, 2016

Conversation

stefanpenner
Copy link
Member

@stefanpenner stefanpenner commented Feb 28, 2016

  • obviate the need to bower install ember.js
  • provide an api for addons to retrieve paths to
    ember.debug.js, ember.prod.js and
    ember-template-compiler.js
  • actually decide on name
  • shims
  • throw if ember-cli version does not support this
  • ember-data fix -> ember-cli-shim check errors if no shim is provided data#4192
  • on-demand build for master, and maybe beta? can happen after
  • release
  • rename to ember-source - ember ember-cli ember-cli-legacy-blueprints
  • backport to release (decided to let it ride the beta cycle so folks can test)

related:

@chadhietala
Copy link
Contributor

chadhietala commented Feb 28, 2016

How about "ember-core"? I liked @mixonic's "bark" analogy which alludes to Ember as the "core competency" of the Ember ecosystem.

As a somewhat tangential comment, I would love to actually see Ember CLI move under the ember repo but released as a separate package. I think this may help with lock stepping projects, overall maintainability, and leverage/integration of tech. A thing working at mega corp has taught me is that for big projects lean towards giant code bases with multiple deployables.

@stefanpenner
Copy link
Member Author

@chadhietala the reason for an alternative name is to create a meta package named ember, which is what you describe (i believe).

@fivetanley
Copy link
Member

I would love to actually see Ember CLI move under the ember repo but released as a separate package

For us on the Ember Data subteam, this seems to create an undue burden on core. We have difficulty getting any settings changed because of the various permission levels Github provides. I think ember-cli remaining its own org empowers its subteam to handle their own tools and collaborators, and core can always commit as they are also in that org.

@stefanpenner
Copy link
Member Author

We should stay in topic. We just need a name for ember-source. So we can reserve ember for a future meta package.

@mixonic
Copy link
Sponsor Member

mixonic commented Feb 28, 2016

@stefanpenner In the 2/19 core meeting we seemed to settle on ember-core as the name. Some discussion under "ember npm package" in the notes.

@homu
Copy link
Contributor

homu commented Mar 15, 2016

☔ The latest upstream changes (presumably #13029) made this pull request unmergeable. Please resolve the merge conflicts.

@Turbo87
Copy link
Member

Turbo87 commented Mar 15, 2016

@stefanpenner what's the state of this PR? can I help to move this forward?

@rwjblue
Copy link
Member

rwjblue commented Mar 15, 2016

Needs a rebase and updating to 'ember-core' as the name. Other than that, I think it's mostly good to land (we can continue to tweak while on master before pushing to NPM).

@stefanpenner
Copy link
Member Author

rebasing now

@stefanpenner
Copy link
Member Author

@rwjblue updated, i think all the remains is:

on-demand build for master, and maybe beta?

Im unsure if this is a blocker, although I can begin working on it over the next few days. (I have small bursts of time)

@@ -58,6 +57,8 @@
"ember-cli-path-utils": "^1.0.0",
"ember-cli-string-utils": "^1.0.0",
"ember-cli-test-info": "^1.0.0",
"ember-cli-valid-component-name": "^1.0.0"
"ember-cli-valid-component-name": "^1.0.0",
"simple-dom": "^0.2.7",
Copy link
Contributor

Choose a reason for hiding this comment

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

did you mean to downgrade this or was this a rebase issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

rebase issue, fixing

@stefanpenner
Copy link
Member Author

Rebase thing likely

@Turbo87
Copy link
Member

Turbo87 commented Apr 12, 2016

@stefanpenner still working on this PR? can I help somehow to bring it over the finish line?

@stefanpenner
Copy link
Member Author

@stefanpenner still working on this PR? can I help somehow to bring it over the finish line?

I'm going to work on it this week, sorry for letting it linger.

@stefanpenner stefanpenner self-assigned this May 12, 2016
@stefanpenner stefanpenner changed the title provide ember-source from NPM provide ember-core from NPM May 20, 2016
@stefanpenner stefanpenner force-pushed the ember-source branch 3 times, most recently from 363ba57 to 218a0c4 Compare May 22, 2016 21:05
@stefanpenner
Copy link
Member Author

@rwjblue this should land on canary confirm?

@mixonic any shims that need fixed/updated/sync'd here is the place let me know.

@rwjblue
Copy link
Member

rwjblue commented May 22, 2016

Yes, let's target canary. It will branch into beta (for 2.7.0) in a couple days.

@stefanpenner
Copy link
Member Author

stefanpenner commented May 22, 2016

@rwjblue this currently wont actually support canary, as it does not yet work with non-prebuilt versions. Its likely ok as it will hit beta soon. I have some other work, then i'll circle back and make this work on canary, but that will most likely result in a refactoring to emberjs-build etc.

'default': Ember.inject.controller
},
'ember-controller/proxy': {
'default': Ember.ArrayProxy
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

This one felt surprising- ArrayProxy in the ember-controller namespace.

Re: ember-cli/ember-cli-shims#60

I guess SortableMixin is in the same boat. Often used for controllers, but the coupling to the namespace seems odd (if we drop controllers, we will be forced to move/rename ArrayProxy and SortableMixin).

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

In fact ObjectProxy is just missing re: ember-cli/ember-cli-shims#36. Perhaps we should add a ember-proxy group 😬

@mixonic
Copy link
Sponsor Member

mixonic commented May 24, 2016

Presented as a checklist so we can check each as "resolved" whether by action or inaction:

Three above:

Two others:

@stefanpenner
Copy link
Member Author

@stefanpenner since we already ship ember-cli-shims by default (in Ember CLI) why not keep them in ember.js but declare them as experimental and not yet public for now? I guess shipping ember.js without the shims might actually break some things unless we want to keep shipping the shims separately via bower.

yes, most likely just do this. Coupling this effort to sorting out the public module API has stifled this effort.

@stefanpenner
Copy link
Member Author

stefanpenner commented Sep 25, 2016

rebased, running tests now.

@stefanpenner
Copy link
Member Author

stefanpenner commented Sep 25, 2016

Looks like only the blueprint tests are failing (on CI). Taking a look

@Turbo87
Copy link
Member

Turbo87 commented Sep 25, 2016

also: "DEPRECATION: Overriding init without calling this._super is deprecated. Please call this._super.init && this._super.init.apply(this, arguments); addon: ember-core" 😉

@stefanpenner
Copy link
Member Author

Alright, green on this end. At one point this did work end-to-end, it may still?

@Turbo87
Copy link
Member

Turbo87 commented Sep 25, 2016

I'd say :shipit: and find out!

@stefanpenner
Copy link
Member Author

stefanpenner commented Sep 25, 2016

Turbo87 someone should try with and without this, and confirm it works. I may have time later this week. If we can make it seamless we have an OK to 🚢

@locks
Copy link
Contributor

locks commented Sep 25, 2016

@stefanpenner Working with a brand new app and super-rentals.

[edit] And a production app of mine.

@rwjblue rwjblue merged commit 454442b into master Oct 7, 2016
@stefanpenner
Copy link
Member Author

14xz1gq

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