BREAKING: WSManListener: Convert to a Class Resource
#106
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #106 +/- ##
====================================
+ Coverage 88% 99% +11%
====================================
Files 3 7 +4
Lines 310 200 -110
====================================
- Hits 274 199 -75
+ Misses 36 1 -35
|
This reverts commit ae0ee81.
This reverts commit a6c859a.
…ghes/WSManDsc into feat/wsman-listener-class
|
I will try to get on this next, as soon as I can. But there is a lot at regular work this week. |
|
That's fine. There is actually something I wanted to discuss/propose around DscResource.Base to enable the use of Enums for optional properties. |
dan-hughes
force-pushed
the
feat/wsman-listener-class
branch
from
e25c270 to
efca431
Compare
|
@johlju this is ready for review again. Uses new base class functionality and preview DocGenerator which has the class enhancements. |
There was a problem hiding this comment.
Reviewed 28 of 35 files at r6, 7 of 7 files at r7, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dan-hughes and @PlagueHO)
source/Classes/020.WSManListener.ps1 line 249 at r3 (raw file):
Previously, dan-hughes (Daniel Hughes) wrote…
I don't believe so, if the port is not supplied, then it is populated with
Get-DefaultPortinGet().GetCurrentState().
I don't think that Port should be set in GetCurrentState, then we will enforce the port even if it was not passes by the user? 🤔 GetCurrentState is not meant to set the instance properties, just return current state. It is also more difficult to understand that Get() affects Set().
If the default port must be used I think it should be set in the Set-function prior to creating the listener. 🤔
source/Private/Find-Certificate.ps1 line 24 at r3 (raw file):
Previously, dan-hughes (Daniel Hughes) wrote…
I'll create issues in the three repos for these.
Great!
There was a problem hiding this comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @johlju and @PlagueHO)
source/Classes/020.WSManListener.ps1 line 249 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
I don't think that Port should be set in GetCurrentState, then we will enforce the port even if it was not passes by the user? 🤔 GetCurrentState is not meant to set the instance properties, just return current state. It is also more difficult to understand that Get() affects Set().
If the default port must be used I think it should be set in the Set-function prior to creating the listener. 🤔
This will include Address too. It can be moved into NewInstance(), which is where it is used.
There was a problem hiding this comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dan-hughes and @PlagueHO)
source/Classes/020.WSManListener.ps1 line 249 at r3 (raw file):
Previously, dan-hughes (Daniel Hughes) wrote…
This will include Address too. It can be moved into NewInstance(), which is where it is used.
Yes, same with Address, missed that. NewInsatnce() sounds good.
Pull Request (PR) description
Convert
WSManListenerto a class resourceRename parameter
DNtoBaseDNThis Pull Request (PR) fixes the following issues
Contributes to #98
Fixes #89
Task list
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
and comment-based help.
This change is