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

Update dependencies with missing ones (Fix #237) #238

Closed
wants to merge 1 commit into from
Closed

Update dependencies with missing ones (Fix #237) #238

wants to merge 1 commit into from

Conversation

SBoudrias
Copy link
Contributor

There was some missing dependencies, and some weirdness in the
package.json. This should be fixed. Bower and grunt-cli have been moved
back to the peerDependencies; if running on travis-CI or similar and
having issues, you should install those two in the pre-build script.

I send a PR because I'm unsure why/how the package.json file have reach the previous state.

@tbranyen
Copy link
Owner

What dependencies are missing? Why are we using peerDependencies other than the semantic value? From what I can tell they don't install or affect anything unless you're installing Backbone Boilerplate through NPM (the generator is the only thing that does this).

devDependencies seems to be the appropriate location for grunt-cli and bower. They are development dependencies and npm's scripts object will put them in the path suitable for CI. The other dependencies you're adding should be covered by the bbb package which has peerDependencies set to grunt-bbb-* tasks.

@SBoudrias
Copy link
Contributor Author

@tbranyen Apparently, the bbb package wasn't enough for #237, and the local installation of grunt-cli was bringing up errors.

peerDependencies are always installed globally and does not enforce a version, not devDependencies. But I changed them mostly because of the issue @RichardCheung had on #237.

@tbranyen
Copy link
Owner

It's because of the brittle nature of load-grunt-tasks. It doesn't check node_modules, instead it checks your package.json.

@SBoudrias
Copy link
Contributor Author

Yup, I think though a fix in load-grunt-task to skip grunt-cli should be ok.

I don't see much advantages to declaring bbb in package.json instead of each grunt-bbb-*. load-grunt-task really ease-up the Gruntfile, I think it should stay if we only encounter those minor problems.

BTW, I'm not sure I see where you'd need to install bower/grunt-cli locally?

There was some missing dependencies, and some weirdness in the
package.json. This should be fixed. Bower and grunt-cli have been moved
back to the peerDependencies; if running on travis-CI or similar and
having issues, you should install those two in the pre-build script.
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 8605b7c on SBoudrias:master into 2750f9f on backbone-boilerplate:master.

@tbranyen
Copy link
Owner

@SBoudrias I posed the question to @cowboy about this and I'd like to follow his advice on how to structure the Gruntfile.

For reference: cowboy/wesbos#5

@tbranyen
Copy link
Owner

Fixed the package.json via: c73b22e

This updates the .travis.yml file to automatically install the correct dependencies exactly per the Markdown.

@tbranyen tbranyen closed this Sep 29, 2013
@tbranyen
Copy link
Owner

Integrated your changes here: be559d0...0ea0358

@SBoudrias
Copy link
Contributor Author

That looks good to me

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

3 participants