-
Notifications
You must be signed in to change notification settings - Fork 8k
Convert -ChildPath parameter to string[] for Join-Path cmdlet
#24677
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
Convert -ChildPath parameter to string[] for Join-Path cmdlet
#24677
Conversation
|
@ArmaanMcleod Since it is a breaking change I suggest to wrap this in an experimental feature (you can find examples in our code). Then I can review and merge since it is already approved by WG. |
|
@iSazonov Interesting. I thought since it was accepted as bucket 3 breaking change it would not need an experimental feature. I did not see that requirement in the issue. In any case would be good to move this along so I will create an experimental feature for this. What should the experimental feature be called? |
|
Although it's a bracket 3, it's still a breaking change. The experimental feature allows us to minimize risks and get feedback faster. |
|
@iSazonov This one actually doesn't make much sense to enable experimental feature, since we are changing the type of |
We have
|
|
@iSazonov Thats really interesting. However even if you do: [Experimental(ExperimentalFeature.PSJoinPathChildArray, ExperimentAction.Show)]
public string[] ChildPath { get; set; }How do you also make the previous parameter: public string ChildPath { get; set; }Also show when the experimental feature is disabled? in C# we can't have two properties have the same name but different type. Maybe I am misunderstanding how this works... |
Yes, it would be too tricky wrap in experimental feature. Let's leave as is. |
| } | ||
| @{ | ||
| Path = 'one' | ||
| ChildPath = [string]::Empty |
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.
We need one more test for empty collection.
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 should already be covered here:
PowerShell/test/powershell/Modules/Microsoft.PowerShell.Management/Join-Path.Tests.ps1
Lines 67 to 71 in 0efc23f
| @{ | |
| Path = 'one' | |
| ChildPath = @() | |
| ExpectedResult = "one${sepChar}" | |
| } |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
📣 Hey @ArmaanMcleod, how did we do? We would love to hear your feedback with the link below! 🗣️ 🔗 https://aka.ms/PSRepoFeedback |
PR Summary
Converted
-ChildPathparameter tostring[]forJoin-Pathcmdlet.PR Context
Fixes #21367
Allows user to give an array of child paths with
-ChildPath. This avoids the extra usage with-AdditionalChildPathwhich can be ignored and makes usage ofJoin-Patheasier.This was discussed to be bucket 3 breaking change.
Example
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.- [x] Issue filed: Document change to
Join-Pathfor using array with-ChildPathparameter. MicrosoftDocs/PowerShell-Docs#11590(which runs in a different PS Host).