fix(libs): 3rd party library install fixes #160
Conversation
|
Btw, thanks to @Brocco for helping me with new prompt texts. |
| @@ -16,7 +16,7 @@ module.exports = Command.extend({ | |||
| run: function (commandOptions, rawArgs) { | |||
| if (!rawArgs.length) { | |||
| var msg = 'The `ng install` command must take an argument with ' + | |||
| 'at least one package name.'; | |||
| 'package name.'; | |||
There was a problem hiding this comment.
"with a/the package name"
missing "a/the"
There was a problem hiding this comment.
Thanks for noticing @cironunes, I fixed it.
|
It looks like there's a lot less tests now. What is the reasoning? |
|
Before it was default that |
|
Got it. Lgtm then! |
| .then(function(resp) { | ||
| return this.announceOKCompletion(); | ||
| }.bind(this)); | ||
| }.bind(this)); | ||
| }, | ||
|
|
||
| installProcedure: function(autoInjection) { | ||
| installProcedure: function() { | ||
| var allPackages = {}; |
There was a problem hiding this comment.
minor: you can improve it to:
var allPackages = { toInstall: [], toProccess: [] };There was a problem hiding this comment.
Improved. Thanks for analyzing the code Ciro, if you find anything else please just add more notes.
| return this.installProcedure(this.autoInjection) | ||
| if (this.autoInjection) { | ||
| if (existsSync(path.resolve(process.cwd(), 'src', 'app.ts'))) { | ||
| var entryPoint = path.resolve(process.cwd(), 'src', 'app.ts'); |
There was a problem hiding this comment.
Can move this line to the top and use existSync on it on line 49?
|
Just added a few minor style improvements. Otherwise LGTM |
|
Fixed. Thanks. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Closes #152 and various other issues installing and injecting 3rd party libs.