quaertym / ember-cli-dependency-checker

Ember CLI addon for checking missing node and bower dependencies before running ember commands
MIT License
40 stars 37 forks source link

make this default in ember-cli #5

Closed stefanpenner closed 9 years ago

stefanpenner commented 9 years ago

@rwjblue

rwjblue commented 9 years ago

yes, but I think the following issues need to be resolved first:

quaertym commented 9 years ago

I hope to finish this by weekend.

stefanpenner commented 9 years ago

@quaertym awesome!

quaertym commented 9 years ago

@stefanpenner @rwjblue do you know any repo for semver checking?

rwjblue commented 9 years ago

node-semver

rwjblue commented 9 years ago

https://github.com/npm/node-semver

quaertym commented 9 years ago

Thanks!

quaertym commented 9 years ago

@rwjblue @stefanpenner #1 and #2 are fixed. Please guide me to fix #4.

quaertym commented 9 years ago

@rwjblue @stefanpenner I know you guys are busy with ember, but can you review v0.0.3 if you have time? Remember that you have to have an unsatisfied dependency in your app.

quaertym commented 9 years ago

@stefanpenner @rwjblue checklist is done except #3 and v0.0.4 is out. I think it is good to include in ember-cli.

quaertym commented 9 years ago

@stefanpenner y u no review?

stefanpenner commented 9 years ago

sorry, scumbag stef... been busy. I have a few minutes now.

stefanpenner commented 9 years ago

@quaertym #4 will be required if we are to include this by default into ember-cli. I really want this in, so if you don't have the time. I will write some test coverage this evening.

quaertym commented 9 years ago

@stefanpenner I am not sure how to best test this. Maybe you can lead and I can add more tests later.

stefanpenner commented 9 years ago

will do

brzpegasus commented 9 years ago

@stefanpenner Have you started any tests? If not, I can try and look into it today. I had tests for that other PR and while they were written for ember-cli, perhaps they could still be of some value here with some adjustments. The code and behavior for the version check is generally the same.

quaertym commented 9 years ago

@brzpegasus Thanks :+1: I borrowed your version checking code. I think it will be great addition to ember-cli.

stefanpenner commented 9 years ago

@stefanpenner Have you started any tests? If not, I can try and look into it today. I had tests for that other PR and while they were written for ember-cli, perhaps they could still be of some value here with some adjustments. The code and behavior for the version check is generally the same

sorry I expected to have time this week, but I have been heads down on some client work.

brzpegasus commented 9 years ago

@quaertym I added part 1 of the tests (see PR #7). Part 2 would be to check Bower, but I'll let you play with that =). One thing that I found a bit odd was that the dependency checker started validating immediately after instantiating it (new EmberCLIDependencyChecker()). I see it was purposefully coded that way; it was just unexpected.

quaertym commented 9 years ago

@brzpegasus Thanks for the great work. What's the better way to do this? Should ember-cli call the checkDependencies method?

rwjblue commented 9 years ago

I think checking on instantiation is desired. The addon will be instantiated near the beginning of each command line invocation. This gives us the best time ( before running a command) to do these checks before missing deps can make us go wrong.

quaertym commented 9 years ago

@rwjblue Is this good enough to include in ember-cli now?

brzpegasus commented 9 years ago

@quaertym Still missing bower tests! See the part 2 I mentioned above.

quaertym commented 9 years ago

@brzpegasus :) I mean after that.

rwjblue commented 9 years ago

Updated my checklist above for inclusion.

quaertym commented 9 years ago

@rwjblue @stefanpenner All the waiting issues are fixed. How do we proceed? This currently only works with ember-cli#master.

rwjblue commented 9 years ago

@quaertym - Submit a PR adding it to the default package.json of the app blueprint in Ember CLI.

quaertym commented 9 years ago

@rwjblue Done. https://github.com/stefanpenner/ember-cli/pull/2454