dsccommunity / FailoverClusterDsc

This module contains DSC resources for deployment and configuration of Windows Server Failover Cluster.
MIT License
60 stars 54 forks source link

xCluster: Enabled localization #118

Closed johlju closed 7 years ago

johlju commented 7 years ago

Pull Request (PR) description

This Pull Request (PR) fixes the following issues: Fixes #83

Task list:


This change is Reviewable

msftclas commented 7 years ago

@johlju, Thanks for having already signed the Contribution License Agreement. Your agreement was validated by Microsoft. We will now review your pull request. Thanks, Microsoft Pull Request Bot

codecov-io commented 7 years ago

Codecov Report

Merging #118 into dev will increase coverage by <1%. The diff coverage is 100%.

Impacted file tree graph

@@         Coverage Diff         @@
##           dev   #118    +/-   ##
===================================
+ Coverage   99%   100%   +<1%     
===================================
  Files        7      7            
  Lines      381    391    +10     
===================================
+ Hits       379    391    +12     
+ Misses       2      0     -2
johlju commented 7 years ago

@PlagueHO do you have time to review this one for me? No hurry.

PlagueHO commented 7 years ago

No problem @johlju - will review tomorrow night after work (bed time now!)

Looks like build is failing though..

johlju commented 7 years ago

Yes. :/ Tried to improve them. They worked when I ran the tests with -Tag Helper but not when all tests ran together... trying to fix the tests now.

johlju commented 7 years ago

Okay, the tests work now, yay! 😄

PlagueHO commented 7 years ago

Looks almost perfect. Just two trivial changes.


Reviewed 2 of 5 files at r1, 2 of 3 files at r2, 1 of 1 files at r3. Review status: all files reviewed at latest revision, 3 unresolved discussions.


Tests/TestHelpers/CommonTestHelper.psm1, line 18 at r3 (raw file):

        [Parameter(Mandatory = $true)]
        [ValidateNotNullOrEmpty()]
        [String]

Change to System.String To be consistent with other code in the module.


Tests/TestHelpers/CommonTestHelper.psm1, line 23 at r3 (raw file):

        [Parameter(Mandatory = $true)]
        [ValidateNotNullOrEmpty()]
        [String]

Change to System.String To be consistent with other code in the module.


Tests/TestHelpers/CommonTestHelper.psm1, line 63 at r3 (raw file):

        [Parameter(Mandatory = $true)]
        [ValidateNotNullOrEmpty()]
        [String]

Change to System.String To be consistent with other code in the module.


Comments from Reviewable

johlju commented 7 years ago

Review status: 4 of 5 files reviewed at latest revision, 3 unresolved discussions.


Tests/TestHelpers/CommonTestHelper.psm1, line 18 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
Change to `System.String` To be consistent with other code in the module.

Done.


Tests/TestHelpers/CommonTestHelper.psm1, line 23 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
Change to `System.String` To be consistent with other code in the module.

Done.


Tests/TestHelpers/CommonTestHelper.psm1, line 63 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
Change to `System.String` To be consistent with other code in the module.

Done.


Comments from Reviewable

PlagueHO commented 7 years ago
:lgtm:

Reviewed 1 of 1 files at r4. Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable