Use arm template parameters as sole input for live test environment variables #2247
Conversation
benbp
added
the
Central-EngSys
|
The following pipelines have been queued for testing: |
| @@ -84,11 +84,13 @@ if (!$PSBoundParameters.ContainsKey('ErrorAction')) { | |||
| $ErrorActionPreference = 'Stop' | |||
| } | |||
|
|
|||
| function Log($Message) { | |||
| function Log($Message) | |||
| { | |||
There was a problem hiding this comment.
Could we leave as-is? I've long practiced JavaScript-like bracketing because for pipelines like when using ForEach-Object, putting a { on the same line means you don't need to use a continuation character. For people who don't know PowerShell as well, this can be very confusing why a script does things differently in different places. Using a consistent bracketing style eliminates that confusion.
There was a problem hiding this comment.
I was trying to buy goodwill from @weshaggard because he likes net-style function syntax and usually makes these comments on my powershell PRs 😄 I don't have a strong opinion either way so I'll let you two duke it out.
There was a problem hiding this comment.
I think your point about script accessibility for powershell newcomers is a good one.
There was a problem hiding this comment.
Basically, to highlight the difference:
filter Foo ([scriptblock] $Script) # filter is like function but defaults to `process {}` instead of `end {}` of pipelines
{
&$Script $_
}
1..10 | Foo {
Write-Host "Got $_"
}
# Or
1..10 | Foo `
{
Write-Host "Got $_"
}Notice the backtick above. Without it, errors abound. So for the sake of consistency and readability, I've gone with JavaScript-style bracing.
There was a problem hiding this comment.
Still, this isn't blocking so I'll unblock, but I think it's worth changing back, @weshaggard. You have previously mentioned about newcomers to the script and why a lot of my PowerShelly shortcuts weren't the best for approachability. I think the same is true here.
There was a problem hiding this comment.
There is definitely some brace issues in PS. I still like having the braces on there own lines for larger blocks, except for when you cannot like the pipelines. I'm also pushing us to not depend on the pipeline syntax in most of our scripts as well as I think they cause more issues then they help with, they are nice at the command line but less so in script files.
As for consistency there is not a great way to be fully consistent but I do want to get some coding style configurations setup for our PS scripts at some point, I believe we have an issue on the backlog for that.
There was a problem hiding this comment.
The pipeline syntax has been there since the beginning. If there are scenarios you'd like me to help with, please point them out. It's been my daily driver - and I'm live in a terminal - since pre-1.0 days, when it was just "Monad". Never had a problem if functions are written correctly.
There was a problem hiding this comment.
Pipelines main goal are for doing things on a single line from the command shell which is great, however when using them in a scripting context they can complicate things. One example is the returning of null, single or list of items when you are filtering. That causes a lot of redundant code and errors when trying to work with lists in different contexts from scripting. That is why I recommend we use the ".Where(...)" on an array over using a pipeline with Where-Object. That ensures that you are always working with a list and you don't end up switching data types in different contexts.
For what it's worth I to have been using PS since it was called monad as my daily shell. At any rate probably not worth having this debate on this particular PR.
|
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 (
|
For sovereign cloud testing, many cloud-specific arm template parameters and environment variables get passed in from an external keyvault source. This makes them hard to manage and track. This PR moves some common live testing values into source control for easier management. Some values are service-specific (like formRecognizerLocation), so perhaps we could consider a service-specific location for them, but I don't want the config layering to get too complex either.
Related:
Azure/azure-sdk-for-java#25301
Azure/azure-sdk-for-net#25250
Azure/azure-sdk-for-python#21693