-
Notifications
You must be signed in to change notification settings - Fork 7
Conversation
I replaced aliases with complete cmdlet name to make this easier to read.
@@ -61,7 +61,7 @@ function Import-Toolkit | |||
} | |||
else | |||
{ | |||
Write-host -ForegroundColor "Import-Toolkit [ToolKit]$path NotFound" | |||
Write-host -ForegroundColor Yellow "Import-Toolkit [ToolKit]$path NotFound" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use Write-Warning per your suggestion?
Is it more advisable to fail if the module file doesn't exist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a good idea to avoid Write-Host. For the reasons, please refer to this blog post - www.jsnover.com/blog/2013/12/07/write-host-considered-harmful/
@jdhitsolutions do you have time to work on this and resolve the review comment? |
Labeling this PR as abandoned since it has gone 14 days or more since the last update. An abandoned PR can be continued by another contributor. The abandoned label will be removed if work on the PR is taken up again. |
Closing this request. Please check whether JeaDsc solves your issues, we're likely to move to that one which is actively maintained by Chris Gardner. |
Fixed Write-Host error for Issue #18. I picked Yellow. Although now that I think about it I wonder why you didn't use Write-Warning.
This change is![Reviewable](https://camo.githubusercontent.com/23b05f5fb48215c989e92cc44cf6512512d083132bd3daf689867c8d9d386888/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)