Skip to content

#112 removed bower as a peer dependency in package.json#136

Merged
afeld merged 3 commits intoafeld:masterfrom
kentmw:master
May 4, 2015
Merged

#112 removed bower as a peer dependency in package.json#136
afeld merged 3 commits intoafeld:masterfrom
kentmw:master

Conversation

@kentmw
Copy link
Copy Markdown

@kentmw kentmw commented May 4, 2015

No description provided.

@gkatsev
Copy link
Copy Markdown
Collaborator

gkatsev commented May 4, 2015

LGTM

@kentmw
Copy link
Copy Markdown
Author

kentmw commented May 4, 2015

I added another patch that allows you to run the tests locally - they fail otherwise when you run npm test

package.json Outdated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I would rather change this line to be bower install && grunt rather than making the install grunt task above.
Unless you update the task to only do bower install the first time.

@kentmw
Copy link
Copy Markdown
Author

kentmw commented May 4, 2015

Better?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

These three things should be installed globally, right?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I imagine bower and grunt-cli should be, but are not guaranteed to be. Having them devDependencies should be safer. Unless they conflict?

Or are you saying, that rather then defining them as devDependencies, instead figure out how to call npm install bower -g?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should be fine as dev deps.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Having them as dev deps allows us to use them in the scripts field without necessarily requiring users install bower and grunt-cli globally.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I like this way. I don't have bower installed globally, and wouldn't like to.

afeld added a commit that referenced this pull request May 4, 2015
#112 removed bower as a peer dependency in package.json
@afeld afeld merged commit 87f747e into afeld:master May 4, 2015
@afeld afeld mentioned this pull request May 4, 2015
@gkatsev
Copy link
Copy Markdown
Collaborator

gkatsev commented May 4, 2015

Also, all our bower dependencies are available on npm, so that could be changed as well at some point.

@kentmw
Copy link
Copy Markdown
Author

kentmw commented May 4, 2015

Any chance we can release v2.0.3 to npm?

@afeld
Copy link
Copy Markdown
Owner

afeld commented May 4, 2015

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.

3 participants