-
Notifications
You must be signed in to change notification settings - Fork 225
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.
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
SqlServerNetwork: Refactored to not load assembly from GAC #1172
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #1172 +/- ##
=====================================
+ Coverage 97% 97% +<1%
=====================================
Files 33 33
Lines 4005 3994 -11
=====================================
- Hits 3916 3907 -9
+ Misses 89 87 -2 |
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.
Reviewed 9 of 9 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions
CHANGELOG.md, line 7 at r1 (raw file):
aspectes
Typo
DSCResources/MSFT_SqlServerNetwork/MSFT_SqlServerNetwork.psm1, line 28 at r1 (raw file):
[ValidateNotNullOrEmpty()] [System.String] $ServerName = $env:COMPUTERNAME,
This parameter can be removed.
DSCResources/MSFT_SqlServerNetwork/MSFT_SqlServerNetwork.psm1, line 225 at r1 (raw file):
SkipClusterCheck = $true
This should only be used if the previous state was disabled ($getTargetResourceResult.IsEnabled -eq $false
), otherwise we never restart the cluster instance.
There is no way in the current implementation to restart a instance where the protocol was enabled from previously been disabled. An issue should be started to track this.
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.
Reviewed 3 of 3 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved
CHANGELOG.md, line 7 at r1 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
aspectes
Typo
Done
DSCResources/MSFT_SqlServerNetwork/MSFT_SqlServerNetwork.psm1, line 28 at r1 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This parameter can be removed.
Done
DSCResources/MSFT_SqlServerNetwork/MSFT_SqlServerNetwork.psm1, line 225 at r1 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
SkipClusterCheck = $true
This should only be used if the previous state was disabled (
$getTargetResourceResult.IsEnabled -eq $false
), otherwise we never restart the cluster instance.There is no way in the current implementation to restart a instance where the protocol was enabled from previously been disabled. An issue should be started to track this.
Done. Opened issue #1174
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.
Mergeíng this when tests passes.
Reviewable status: complete! all files reviewed, all discussions resolved
Pull Request (PR) description
SkipClusterCheck
andSkipWaitForOnline
. This was to support more aspectsof the resource SqlServerNetwork.
enabling and disabling the protocol.
(issue SqlServerNetwork: Integration test is missing for this resource #751).
This Pull Request (PR) fixes the following issues
Helps with issue #1151
Fixes #1051 (closing this since the issue is around code that has been removed).
Fixes #751
Fixes #581 (closing this since the issue is around code that has been removed).
Task list
should say what was changed, and how that affects users (if applicable).
and comment-based help?
See DSC Resource Testing Guidelines.
See DSC Resource Testing Guidelines.
DSC Resource Style Guidelines
and Best Practices?
This change is