Add back ADSI and WMI type accelerators - #7085
Conversation
4774745 to
3836e93
Compare
|
I believe the problem is we haven't published an updated pwrshplugin.dll yet so System.Management.dll and System.DirectoryServices.dll aren't in the trusted list and thus fails to initialize on the remote side. I'll work on getting pwrshplugin.dll published. |
There was a problem hiding this comment.
Can sort the using list?
There was a problem hiding this comment.
I'll put the two here in alpha order. Won't touch the ones above.
There was a problem hiding this comment.
The CommaDelimitedStringCollection class is in .Net Core 2.0+.
https://apisof.net/catalog/System.Configuration.CommaDelimitedStringCollection
There was a problem hiding this comment.
Can we sort the using list?
There was a problem hiding this comment.
Can we sort the using list?
There was a problem hiding this comment.
We should do a global pass if we want them all sorted. Separate PR.
There was a problem hiding this comment.
I ask only because you have already edited the list.
There was a problem hiding this comment.
I suppose I could update it just for this file since I'm touching this part anyways.
There was a problem hiding this comment.
Maybe use Compile Remove in csproj file?
|
Seems we add back 5 type accelerators - please update the PR description. |
Steve Lee (SteveL-MSFT)
changed the title
3836e93 to
4643db4
Compare
There was a problem hiding this comment.
We should enable the test on Windows and correct $totalAccelerators = 95 above.
There was a problem hiding this comment.
Missed the -Skip part of this test. Will fix. Not sure what you mean by "correct $totalAcclerators = 95 above" as 95 is the correct number.
There was a problem hiding this comment.
Seems we add 5 accelerators on Windows. I wonder why the count is increased only +1
52fdd1f to
0bf9fb1
Compare
3d11205 to
a89c51d
Compare
|
Ilya (@iSazonov) can you formally review and signoff? |
Paul Higinbotham (PaulHigin)
left a comment
Paul Higinbotham (PaulHigin)
left a comment
There was a problem hiding this comment.
LGTM
update count of accelerators in test
a89c51d to
12c2db3
Compare
|
I still have a concern about my comment
|
|
Ilya (@iSazonov) the count didn't increase by +1, if you look at line 381, the number w/o the Windows specific ones is 91. I added 5, so the total with the new ones is 96 on line 385. The original code was wrong as it only did +4 and should have been +5. |
4 participants
PR Summary
Now that we include Microsoft.Windows.Compatibility assemblies with PowerShell Core 6.1, we can add back the [adsi], [adsisearcher], [wmi], [wmiclass], and [wmisearcher] accelerators putting back code that already exists.
Fix #3266
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:to the beginning of the title and remove the prefix when the PR is ready.[feature]if the change is significant or affects feature tests