Join GitHub today
GitHub is home to over 36 million developers working together to host and review code, manage projects, and build software together.
Sign upAdd `Enable-ExperimentalFeature` and `Disable-ExperimentalFeature` cmdlets #8318
Conversation
SteveL-MSFT
added
the
Breaking-Change
label
Nov 20, 2018
SteveL-MSFT
assigned
daxian-dbw
Nov 20, 2018
SteveL-MSFT
requested review from
daxian-dbw and
PaulHigin
Nov 20, 2018
SteveL-MSFT
requested a review
from
BrucePay
as a
code owner
Nov 20, 2018
SteveL-MSFT
added
the
Review - Committee
label
Nov 20, 2018
rjmholt
added
the
Documentation Needed
label
Nov 20, 2018
rjmholt
reviewed
Nov 20, 2018
|
Have left some small comments, but it all looks pretty good to me. We should also make sure we start the documentation process before merging this PR. |
...on/engine/ExperimentalFeature/EnableDisableExperimentalFeatureCommand.cs Outdated
...on/engine/ExperimentalFeature/EnableDisableExperimentalFeatureCommand.cs Outdated
src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs Outdated
src/System.Management.Automation/engine/PSConfiguration.cs Outdated
src/System.Management.Automation/engine/PSConfiguration.cs Outdated
PaulHigin
requested changes
Nov 20, 2018
| features.Remove(featureName); | ||
| } | ||
|
|
||
| WriteValueToFile<string[]>(scope, "ExperimentalFeatures", features.ToArray()); |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
SteveL-MSFT
Nov 20, 2018
Author
Member
Seems like this should be a separate issue for WriteValueToFile(). Opened #8325
This comment has been minimized.
This comment has been minimized.
PaulHigin
Nov 20, 2018
Contributor
Actually, I disagree. We can save even running WriteValueToFile() code, which involves opening the file and reading data, by simply keeping track of whether we actually have a change to record.
This comment has been minimized.
This comment has been minimized.
PaulHigin
Nov 20, 2018
Contributor
Unless we are concerned with file contention. If so, then I assume the last write to should win, and it makes sense to ensure the file actually reflects the current config state.
SteveL-MSFT
force-pushed the
SteveL-MSFT:experimental-feature-cmdlets
branch
to
c8743f3
Nov 20, 2018
PaulHigin
approved these changes
Nov 20, 2018
|
LGTM |
iSazonov
added
CL-BreakingChange
CL-Engine
labels
Nov 21, 2018
This comment has been minimized.
This comment has been minimized.
|
Can we assign two |
joeyaiello
referenced this pull request
Nov 26, 2018
Merged
Draft of experimental feature user experience RFC #148
SteveL-MSFT
added
Committee-Reviewed
and removed
Review - Committee
labels
Nov 28, 2018
This comment has been minimized.
This comment has been minimized.
|
@PowerShell/powershell-committee reviewed this as part of the RFC review and approves of the breaking change and current cmdlet design |
SteveL-MSFT
added some commits
Nov 5, 2018
SteveL-MSFT
force-pushed the
SteveL-MSFT:experimental-feature-cmdlets
branch
from
c8743f3
to
4ba6cbc
Nov 29, 2018
SteveL-MSFT
force-pushed the
SteveL-MSFT:experimental-feature-cmdlets
branch
from
4ba6cbc
to
6727dd7
Nov 29, 2018
SteveL-MSFT
removed
the
Documentation Needed
label
Nov 29, 2018
This comment has been minimized.
This comment has been minimized.
|
Doc PR submitted and linked |
iSazonov
reviewed
Nov 30, 2018
|
We lost the comment |
iSazonov
and others
added some commits
Nov 30, 2018
iSazonov
and others
added some commits
Nov 30, 2018
iSazonov
approved these changes
Nov 30, 2018
|
@SteveL-MSFT You skipped two my comments for |
iSazonov
and others
added some commits
Nov 30, 2018
This comment has been minimized.
This comment has been minimized.
|
@iSazonov it was hidden under GitHub collapsing it. Thanks for calling it out. Fixed. |
This comment has been minimized.
This comment has been minimized.
|
@SteveL-MSFT HelpUri do not pass tests. |
iSazonov
reviewed
Dec 1, 2018
|
It seems we need remove the cmdlets from release versions. |
daxian-dbw
requested changes
Dec 4, 2018
vexx32
and others
added some commits
Dec 4, 2018
SteveL-MSFT
requested review from
adityapatwardhan,
anmenaga and
TravisEz13
as
code owners
Dec 4, 2018
daxian-dbw
approved these changes
Dec 4, 2018
This comment has been minimized.
This comment has been minimized.
|
@SteveL-MSFT One test for |
This comment has been minimized.
This comment has been minimized.
|
@daxian-dbw I forgot to update the tests to reflect that the cmdlet no longer outputs the feature object. Will update. |
SteveL-MSFT commentedNov 20, 2018
•
edited
PR Summary
Add
Enable-ExperimentalFeatureandDisable-ExperimentalFeaturecmdlets. Remove-ListAvailablefromGet-ExperimentalFeature(Breaking Change). Add ArgumentCompleter forGet-ExperimentalFeaturecmdlet. Refactor some existing Experimental Feature tests. MakeConfigScopepublic and renamedSystemWidetoAllUsers.Implements PowerShell/PowerShell-RFC#148
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