New archi take2 #93
New archi take2 #93
Conversation
| return; | ||
| } | ||
|
|
||
| pluginsHelper._isPackageExists = function(where, packageName) { |
There was a problem hiding this comment.
looks good to me, you're just missing the change from my last commit that would make loadPackage much simpler, make sure plug-ins are always up-to-date, and remove the need for _isPackageExists() entirely.
There was a problem hiding this comment.
I was thinking this way. Don't check the installation all the times, but do it only when passed an option. Hope this install command does some network activity.
There was a problem hiding this comment.
Npm is pretty clever… on my devel I tried it my way, and if there's nothing to update, you don't even notice it's running.
There was a problem hiding this comment.
(except it complains that there's no README.md and there doesn't seem to be a way to shut that up 😼 but speed-wise it's pretty invisible)
There was a problem hiding this comment.
okay. if so that's nice. I'll update it.
| } | ||
| } | ||
|
|
||
| var needNpmInstall = options.argv.npmInstall; |
There was a problem hiding this comment.
@lalomartins see: we can force install with -N(--need-install) option. I think this is a better approach.
There was a problem hiding this comment.
I just don't think it's necessary. It puts extra demand on the user/developer for no benefit, since the npm run is invisibly fast.
There was a problem hiding this comment.
I know(I tried). the issue comes when he uses a github repo or a tarball as the dependancy.
There was a problem hiding this comment.
Hm. That's a bit of a corner case… then by the design principle of “unbreak my app” (a.k.a. sane default), I'd say it's better to always update unless an option tells us not to. Then of course if you need to use dependencies that make things slow, you can put that option in your config file.
There was a problem hiding this comment.
okay. good idea. I'll do it.
| process.chdir(testsPath); | ||
| npm.load({verbose: process.argv.verbose}, function(error, Npm) { | ||
| //a hack to hide warnning | ||
| Npm.registry.log.warn = function() {}; |
There was a problem hiding this comment.
@lalomartins a hack to remove warn messages. I hope I can merge this now.
@lalomartins I made few changes to the logic and moved all plugins related logic into a separate module. What do you think?