Repository navigation
Conversation
PowerShell is not case sensitive, so the parameter and the variable were considered to be the same. fixes Set-D365WebServerType is not working Fixes d365collaborative#927
This pull request was automatically created by the d365fo.tools-Generate-Text action'
|
@Splaxi Pinging you as reviewer since I added a reference implementation of the ShouldProcess support that I would like to see more in our cmdlets. |
| ) | ||
|
|
||
| begin { | ||
| if ($Force -and -not $PSBoundParameters.ContainsKey('Confirm')) { |
There was a problem hiding this comment.
didn't we have this discussion once? That contains would fire if -Confirm:$false?
We should be able to have it be simpler with $Confirm
I'm not entirely sure
There was a problem hiding this comment.
@Splaxi
I was following the guidance in ShouldProcess on that. -Confirm:$false would be the one of the intended ways to override the confirmation behavior, the other would be changing the $ConfirmPreference. But since users are more familiar with the -Force parameter, they tend to use that in an attempt to get rid of confirmations. This basically makes it so that -Force works like -Confirm:$false. I think this works in this case because -Force is not used to control other behavior.
In any case, since the ConfirmImpact is set to 'Low', most users will never run into unwanted confirmations. Either you set $ConfirmPreference to 'Low' before running the command (which means you seem to know what you are doing) or you are using the -Confirm switch (out of curiosity or because you know what you are doing).
|
I see some changes to the CBH. Maybe you and I are running different mechanics - that formats. Let's align, so we don't see drift like that show up all the time... |
I'm using https://github.com/d365collaborative/d365fo.tools/actions/workflows/update-generated-text.yml. Might be because it is running on a unix agent that causes the diffs. |
Unix -> Most likely is the cause |
|
@Splaxi Are there any changes you would like to see before approval? I will take a look at the CBH drifts at a later time, but I don't think it is necessary to block this pr for that. We've been living with the drifts for a while now, a few more are acceptable I think. |
| .PARAMETER Confirm | ||
| Switch parameter to require confirmation before making changes. | ||
|
|
||
| .PARAMETER WhatIf | ||
| Switch parameter to show what would happen if the cmdlet runs, without making any changes. | ||
|
|
||
| .PARAMETER Force | ||
| Switch parameter to force the operation without confirmation. | ||
|
|
There was a problem hiding this comment.
I would "hate" that we would have to have documentation for all 3 standard parameter, across all cmdlets.
There was a problem hiding this comment.
@FH-Inway - If we could settle without these, at least over time - then I'm happy.
We'll have to face what operations / actions should be medium or high - so people can accept things, without having to manage Confirm.
If we start with low - then all people will have non-breaking changes, while you can set "low" on your profile and you will be prompted.
Then over time, we can gradually promote different functions / cmdlets, to different levels.
Splaxi
left a comment
Splaxi
left a comment
There was a problem hiding this comment.
Take the feedback as non-blocking
Fixes #429 by renaming the internal variable for the xml node where the
RuntimeHostTypeis stored.Also enhances the
Set-D365WebServerTypecmdlet by adding support for PowerShell's-WhatIf,-Confirm, and a new-Forceparameter, improving safety and flexibility when making configuration changes.