Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verbose install parameter #451

Merged
merged 1 commit into from
Mar 2, 2017
Merged

Verbose install parameter #451

merged 1 commit into from
Mar 2, 2017

Conversation

tolgabalci
Copy link
Contributor

When troubleshooting install issues, it would be nice to be able to call:
.\install.ps1 -Verbose

Alternative for seeing verbose output from install.ps1 is:

$VerbosePreference = "Continue"
.\install.ps1

Another option would be to add SupportsShouldProcess binding, but then Import-Module also displays verbose output and it is pretty chatty. I am assuming that is why WhatIf was also added separately.

I wasn't sure though the order you would like the Verbose parameter declared in and then later used when calling Add-PoshGitToProfile. Let me know if you would like for me to change the orders.

@dahlbyk
Copy link
Owner

dahlbyk commented Mar 2, 2017

Thanks for the contribution!

Another option would be to add SupportsShouldProcess binding, but then Import-Module also displays verbose output and it is pretty chatty. I am assuming that is why WhatIf was also added separately.

Honestly, SupportsShouldProcess isn't used because I didn't know about it at the time that I added -WhatIf six years ago.

That said, it doesn't look like Import-Module actually SupportsShouldProcess. It does support -Verbose, and I was going to suggest passing your new $Verbose flag through to it - yeah it's chatty, but that's kind of the point of -Verbose, no?

I wasn't sure though the order you would like the Verbose parameter declared in and then later used when calling Add-PoshGitToProfile. Let me know if you would like for me to change the orders.

I don't care about the order, but I do wonder if install.ps1 should just use [CmdletBinding()] so we don't have to explicitly define or pass along -WhatIf or -Verbose. Thoughts?

Copy link
Collaborator

@rkeithhill rkeithhill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a such a simple script, this LGTM. Probably not worth [CmdletBinding()].

@dahlbyk dahlbyk merged commit 7f3a7fb into dahlbyk:master Mar 2, 2017
@dahlbyk dahlbyk added this to the v0.7.1 milestone Mar 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants