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

Ember Add-on now depends on bower instead of using pre-built dist #2249

Merged
merged 1 commit into from
Sep 8, 2014

Conversation

jayphelps
Copy link
Contributor

Long story why...Ask me or @rwjblue sometime and we'll talk your ear off for an hour......

@@ -3,7 +3,7 @@
module.exports = {
name: 'ember-data',
init: function() {
this.treePaths.vendor = '../dist';
this.treePaths.bower = '../dist';
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment to indicate that we use the bower tree so that consuming applications can override?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So basically move this up?

Copy link
Member

Choose a reason for hiding this comment

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

Crap, I didn't see that.

@@ -19,8 +19,8 @@ module.exports = {
// but still allow consumers to bower install their own version that
// will override ours.
this.app.import({
development: 'vendor/ember-data/ember-data.js',
production: 'vendor/ember-data/ember-data.prod.js'
development: 'bower/ember-data/ember-data.js',
Copy link
Member

Choose a reason for hiding this comment

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

needs to be app.bowerDirectory + '/ember-data/ember-data.js'

@jayphelps jayphelps force-pushed the patch-2 branch 2 times, most recently from 9e9d866 to 02ead0c Compare September 8, 2014 02:00
@jayphelps jayphelps changed the title add-on now uses bower tree Ember Add-on now depends on bower instead of using pre-built dist Sep 8, 2014
@jayphelps jayphelps force-pushed the patch-2 branch 4 times, most recently from 753e673 to dbcbb4f Compare September 8, 2014 02:13
@jayphelps
Copy link
Contributor Author

@rwjblue This should be GTG.

"scripts": {
"postinstall": "bower install",
"prepublish": "ember build --output-path=dist/ember-data/ --environment production",
Copy link
Member

Choose a reason for hiding this comment

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

Can you add the bower install into the prepublish?

prepublish is ran when running npm install (without a package name argument), and before publishing to npm. Adding prepublish as bower install will still leave the instructions in the README on how to build this project work (basically just git clone && npm install && npm start).

…bower dep instead of including a pre-built version
"scripts": {
"postinstall": "bower install",
"prepublish": "ember build --output-path=dist/ember-data/ --environment production",
"prepublish": "bower install",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rwjblue 👍/👎

Copy link
Member

Choose a reason for hiding this comment

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

👍

rwjblue added a commit that referenced this pull request Sep 8, 2014
Ember Add-on now depends on bower instead of using pre-built dist
@rwjblue rwjblue merged commit 0776c10 into emberjs:master Sep 8, 2014
@rwjblue rwjblue mentioned this pull request Sep 8, 2014
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.

2 participants