Skip to content

List experimental feature for NewPSBreakpoint in Module manifest for Windows #9907

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.

Sign up for GitHub

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

Closed
TylerLeonhardt wants to merge 1 commit into from
Closed

List experimental feature for NewPSBreakpoint in Module manifest for Windows #9907

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/Modules/Windows/Microsoft.PowerShell.Utility/Microsoft.PowerShell.Utility.psd1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,14 @@ FunctionsToExport = @()
AliasesToExport = @('fhx')
NestedModules = @("Microsoft.PowerShell.Commands.Utility.dll")
HelpInfoURI = 'https://go.microsoft.com/fwlink/?linkid=855960'
PrivateData = @{
PSData = @{
ExperimentalFeatures = @(
@{
Name = 'Microsoft.PowerShell.Utility.PSDebugRunspaceWithBreakpoints'
Description = "Enables the New-PSBreakpoint cmdlet and the -Breakpoint parameter on Debug-Runspace to set breakpoints in another Runspace upfront."
Copy link
Member

@daxian-dbw daxian-dbw Jun 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New-PSBreakpoint is an experimental feature, then it should be exposed only if the feature is turned on, right?
If that's true, then the CmdletsToExport should be conditional based on $EnabledExperimentalFeatures -contains 'Microsoft.PowerShell.Utility.PSDebugRunspaceWithBreakpoints'

Copy link
Member

@daxian-dbw daxian-dbw Jun 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a side question, do we really need this to be an experimental feature? What's the thoughts on putting it behind experimental flag? (design may be change? might cause regression?)

Copy link
Contributor

@KirkMunro KirkMunro Jul 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daxian-dbw: I'm glad it is listed as experimental, because there are some challenges with the design. See: #8923 (comment).

Copy link
Contributor

@KirkMunro KirkMunro Jul 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, the New-PSBreakpoint cmdlet has been removed in a PR that should be submitted today. I've also removed the experimental feature flag, because I think it is not needed anymore. We can work out details in that PR, and I'll happily add back the experimental feature flag (fully, including what was missed the first time) if necessary. This PR can probably be closed once I have that PR open, so I'll comment back here soon.

Copy link
Member Author

@TylerLeonhardt TylerLeonhardt Jul 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've also removed the experimental feature flag, because I think it is not needed anymore.

The experimental feature flag is still needed. That will be my first piece of feedback.

Copy link
Contributor

@KirkMunro KirkMunro Jul 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But you haven't even seen the code... 😛

}
)
}
}
}