-
Notifications
You must be signed in to change notification settings - Fork 183
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
Remove old Update-Change-Log.ps1 #1301
Remove old Update-Change-Log.ps1 #1301
Conversation
The following pipelines have been queued for testing: |
@@ -12,8 +12,8 @@ param ( | |||
[String]$ServiceDirectory, | |||
[Parameter(Mandatory = $true)] | |||
[String]$PackageName, | |||
[String]$Unreleased=$True, | |||
[String]$ReplaceLatestEntryTitle = $False, | |||
[String]$Unreleased = "true", #Argument is string becasue of the different ways the script is called in the various repos. |
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.
Curious about this, why can't you do this?
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.
This is because of how the script is called here. PowerShell in converting to Boolean results in true for both "true" and "false"
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.
I'd normally use a [switch]
type for this kinds of options in PowerShell, example:
function Do-Stuff
{
param(
[switch]$FooSwitch
)
Write-Host $FooSwitch
}
When you invoke this function you can do this:
Do-Stuff
Output: False
Do-Stuff -FooSwitch
Output: True
Do-Stuff -FooSwtich:$false
Output: False
Do-Stuff -FooSwitch:$([System.Boolean]::Parse("false"))
Output: False
Do-Stuff -FooSwitch:$([System.Boolean]::Parse("true"))
Output: True
In short PowerShell provides the mechanisms to declare these kinds of switches and pass in true-false values.
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.
Oh, I see.
Do you know how the switch will be invoked from JS
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.
Yes I expect the switch to work fine from anywhere (Note that -FooSwitch:0/1 also works). The key with using a switch is to use a term that means the variable defaults to false and so folks will only ever pass it if they want to set it to true.
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 looks like the array that is passed into the spawn command is just the argument list. But it looks like pwsh.exe
has a nice little affordance where you can invoke it like this:
pwsh.exe [script] [args]
For example:
pwsh.exe Do-Stuff.ps1 -FooSwitch:false
(note you don't need to do the [System.Boolean] thing.
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.
@weshaggard Unfortunately -FooSwitch:0/1
does not work. This does work though -FooSwitch:$true
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.
LGTM - Make sure this goes in after your other updates to the language repos.
Hello @azure-sdk! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
/check-enforcer override |
This reverts commit b967cb6.
Reverts #1301 Update-Change-Log.ps1 is still needed for some other Repos.
eng\common\Update-Change-Log.ps1