GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Expose all cmdlets from PSDiagnostics if logman.exe is available by SteveL-MSFT · Pull Request #8366 · PowerShell/PowerShell

Conversation

Steve Lee (SteveL-MSFT) commented Nov 30, 2018 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

PR Summary

PSDiagnostics had outdated logic to expose cmdlets for PSCore5 where logman.exe didn't exist, but this is no longer the case with PSCore6 on full Windows. Change logic to check for existence of logman.exe in path rather than assume Core vs Desktop PowerShell. Deferring adding tests for these cmdlets as it seems we don't have any tests for PSDiagnostics module: #8367

Fix #8339

PR Checklist

PR has a meaningful title
- Use the present tense and imperative mood when describing your changes
Summarized changes
Change is not breaking
Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
This PR is ready to merge and is not Work in Progress.
- If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.
User-facing changes
- Not Applicable
- OR
- User-facing Documentation needed
  - Issue filed - Issue link:
Testing - New and feature
- Not Applicable or can only be tested interactively
- OR
- Make sure you've added a new test if existing tests do not effectively test the code changed
  - Add [feature] if the change is significant or affects feature tests

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          change logic to expose compatible cmdlets in psdiagnostics

8ab792f

Steve Lee (SteveL-MSFT) assigned Ilya (iSazonov)

Nov 30, 2018

Ilya (iSazonov) approved these changes

Nov 30, 2018

View reviewed changes

Ilya (iSazonov) left a comment

Copy link

Copy Markdown

Collaborator

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.

Steve Lee (@SteveL-MSFT) Can your team add CI for Nano/IoT/Windows 7 x32 if it is important?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/Modules/Windows/PSDiagnostics/PSDiagnostics.psm1 Resolved

Uh oh!

There was an error while loading. Please reload this page.

Ilya (iSazonov) added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label

Nov 30, 2018

Steve Lee (SteveL-MSFT) commented Nov 30, 2018

Copy link

Copy Markdown

Member Author

Ilya (@iSazonov) we're working on improving our release automation which includes running tests daily on all supported distros/skus. Don't think Nano nor Win10 IoT will be there initially. I expect Win7, but not x86 initially if that's what you mean.

iSazonov reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          address Ilya's feedback

3ad8b7c

Ilya (iSazonov) reviewed

Nov 30, 2018

View reviewed changes

Comment thread

src/Modules/Windows/PSDiagnostics/PSDiagnostics.psm1

		{
		Export-ModuleMember Start-Trace, Stop-Trace, Enable-WSManTrace, Disable-WSManTrace, Enable-PSTrace, Disable-PSTrace, Enable-PSWSManCombinedTrace, Disable-PSWSManCombinedTrace, Get-LogProperties, Set-LogProperties
		}
		if (Get-Command logman.exe -Type Application -ErrorAction SilentlyContinue)

Ilya (iSazonov) Nov 30, 2018

Copy link

Copy Markdown

Collaborator

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.

Seems a comment about Nano/IoT will be still useful.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ilya (iSazonov) Dec 4, 2018

Copy link

Copy Markdown

Collaborator

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.

Steve Lee (@SteveL-MSFT) Could you clarify whether this comment is needed?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Steve Lee (SteveL-MSFT) Dec 4, 2018

Copy link

Copy Markdown

Member Author

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.

The comment still exists on line 446

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

iSazonov reacted with thumbs up emoji

All reactions

👍 1 reaction

Dongbo Wang (daxian-dbw) approved these changes

Dec 4, 2018

View reviewed changes

Ilya (iSazonov) merged commit 4c24f71 into PowerShell:master

Dec 5, 2018

Sean Wheeler (sdwheeler) mentioned this pull request

Dec 5, 2018

Sdw diag 62 MicrosoftDocs/PowerShell-Docs#3379

Closed

2 of 9 tasks

Steve Lee (SteveL-MSFT) deleted the psdiagnostics-cmdlets branch

December 5, 2018 21:36

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request

Aug 6, 2026


          Expose all cmdlets from PSDiagnostics if logman.exe is available (Pow…

fb17544

…erShell#8366)

Reviewers

Ilya (iSazonov)

iSazonov approved these changes

Dongbo Wang (daxian-dbw)

daxian-dbw approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

Ilya (iSazonov)

Labels

CL-General

Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

PSDiagnostics does not properly export all cmdlets in PSv6

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

Steve Lee (SteveL-MSFT) commented Nov 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

Ilya (iSazonov) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Steve Lee (SteveL-MSFT) commented Nov 30, 2018

Uh oh!

Ilya (iSazonov) Nov 30, 2018

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) Dec 4, 2018

Choose a reason for hiding this comment

Uh oh!

Steve Lee (SteveL-MSFT) Dec 4, 2018

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Steve Lee (SteveL-MSFT) commented Nov 30, 2018 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.