feedback

[stefanpenner]

my incredibly version pedantic feedback.

• [x] expose https://github.com/quaertym/ember-cli-dependency-checker/blob/v0.0.5/index.js#L139-L152 so it can be imported and tested against a69312c6332e770825ffe3e0f53db2cb4cc7d5e8
• [x] ductype DependencyError isDependencyError 5b694d1f17651be56e1ad2ca13b2027ed71e613c (please verify this is what you meant)
• [ ] i feel this should be an ember-cli thing, and we should always include the stack https://github.com/quaertym/ember-cli-dependency-checker/blob/v0.0.5/index.js#L143
• [x] use Object.create(Error.prototype) https://github.com/quaertym/ember-cli-dependency-checker/blob/v0.0.5/index.js#L151 5ab831c663b4220cedf80833e486ae812be2246f
• [x] https://github.com/quaertym/ember-cli-dependency-checker/blob/v0.0.5/index.js#L19-L21 should be defined in the outer context and as function isUnsatisfied(pkg) { ... } 6f6cfebc855545a298cc3ccf3c6ba019306a903f
• [x] it might be strange that the EmberCLIDependencyChecker constructor has sideaffects beyond setting up the object https://github.com/quaertym/ember-cli-dependency-checker/blob/v0.0.5/index.js#L14 (but i can easily be convinced this is what we want)
• [x] will https://github.com/quaertym/ember-cli-dependency-checker/blob/v0.0.5/index.js#L30-L31 be a problem if people bower link or npm link to different versions?
• [x] it seems strange that an instance method, doesn't merely reach into is own object to retrieve the project instead project is also passed in as an argument. This goes to all instance methods, that also consume project. It is then unclear to me, if the project passed in can differ from the one passed to the constructor. And if that is the case, why bother with a stateful object at all? a436140290a604b53d5697f65a6b8d04d58ca006
• [x] https://github.com/quaertym/ember-cli-dependency-checker/blob/v0.0.5/index.js#L73-L75 should be defined in the outer context 13e116ed1b9af1c78b22c8328d38f98033eae6fc

[stefanpenner]

@rwjblue should confirm, but it should also be possible for us to delete extra directories not actually used by the addon. If not, we should open an ember-cli issue so that it becomes possible.

Although the addon generator spews out lots of folders to make it easy for the general case, specialized plugins shouldn't include the same directory soup.

I have opened a tracking issue: https://github.com/stefanpenner/ember-cli/issues/2404

[stefanpenner]

@quaertym I will test this addon in some of my apps after work today :)

[quaertym]

@stefanpenner Please do, my team greatly benefitted from this.

I tag the item with the commit that fixes it above. Some them are not very clear though.

[quaertym]

@stefanpenner Please review completed items if you have time.

For the open items:

i feel this should be an ember-cli thing, and we should always include the stack

Does this contradict with what @rwjblue suggested here https://github.com/quaertym/ember-cli-dependency-checker/issues/4?

it might be strange that the EmberCLIDependencyChecker constructor has sideaffects beyond setting up the object

When the object is constructed, we want to immediately check dependencies. Is there a better way to do this? (edit: I think this is what we want.)

will https://github.com/quaertym/ember-cli-dependency-checker/blob/v0.0.5/index.js#L30-L31 be a problem if people bower link or npm link to different versions?

This is the output when you specified ember-cli v0.1.1 in your package.json and npm link ember-cli v0.1.2: (edit: It seems alright.)

version: 0.1.2-master-602a1c4558

Missing npm packages: 
Package: ember-cli
  * Specified: 0.1.1
  * Installed: 0.1.2

Run `npm install` to install missing dependencies.

[quaertym]

@rwjblue any comments on @stefanpenner 's feedback and the last open item?

[quaertym]

@stefanpenner any comments about the last open item?

[stefanpenner]

The last open item would be wonderful but doesn't block us. Some future refactoring can tackle it