PlagueHO commented 7 years ago

This PR adds support for specifying the disk to work with using the Disk GUID. This is a unique value that is only assigned to the disk once it is initialized (a partition table is created). It will work with either MBR or GPT partition formats.

I've applied auto-formatting on any files I touched.

Tagging @Zuldan

This change is

codecov-io commented 7 years ago

Codecov Report

Merging #107 into dev will decrease coverage by <1%. The diff coverage is 100%.

@@        Coverage Diff         @@
##           dev   #107   +/-   ##
==================================
- Coverage   95%    94%   -1%     
==================================
  Files        5      5           
  Lines      687    671   -16     
==================================
- Hits       657    636   -21     
- Misses      30     35    +5

johlju commented 7 years ago

Reviewed 15 of 19 files at r1, 6 of 6 files at r2. Review status: all files reviewed at latest revision, 9 unresolved discussions.

.markdownlint.json, line 4 at r2 (raw file):

    "default": true,
    "MD029": {
        "style": "ordered"

Should be "one" now.

CHANGELOG.md, line 15 at r2 (raw file):

- Added .markdownlint.json file to configure markdown rules to validate.
- Clean up Badge area in README.MD - See [Issue 110](https://github.com/PowerShell/xStorage/issues/110)
- Disabled MD013 rule checking to enable badge table.

Wouldn't it be better to use headers instead to have this enable? See example in xFailoverCluster and xSQLServer.

Ps. I don't like that there is no way to split a table row on several rows. :disappointed: Ds.

Tests/Integration/MSFT_xWaitForDisk.Integration.Tests.ps1, line 135 at r2 (raw file):

                        -ConfigurationData $configData
                    Start-DscConfiguration -Path $TestDrive -ComputerName localhost -Wait -Verbose -Force
                } | Should not throw

Should Not Throw (upper S, N and T). All other tests too.

Tests/TestHelpers/CommonTestHelper.psm1, line 125 at r2 (raw file):

        # The disk will be initialized with GPT
        $diskPartScript += @"

Remove blank row here? If it is needed for the script to work, than make a comment that it is needed.

Tests/Unit/MSFT_xDisk.Tests.ps1, line 1929 at r2 (raw file):

                $script:result = $null

                It 'calling test Should Not Throw' {

It 'Should not throw an error' seems better.

Tests/Unit/MSFT_xDiskAccessPath.Tests.ps1, line 1429 at r2 (raw file):

                $script:result = $null

                It 'calling test Should Not Throw' {

Same as previous comment.

Tests/Unit/MSFT_xWaitForDisk.Tests.ps1, line 121 at r2 (raw file):

                $script:result = $null

                It 'Should Not Throw' {

Same as previous comment.

Tests/Unit/MSFT_xWaitForDisk.Tests.ps1, line 198 at r2 (raw file):

                    -Verifiable

                It 'Should Not Throw' {

Same as previous comment. There are a lot of these (maybe even more of these in unchanged code?

Tests/Unit/StorageDsc.Common.Tests.ps1, line 184 at r2 (raw file):

                }

                It 'Should call exepced mocks' {

Typo (and all others below too)

Comments from Reviewable

PlagueHO commented 7 years ago

Sorry about all the changes here - it is all working towards HQRM on this module as well.

Review status: 12 of 26 files reviewed at latest revision, 10 unresolved discussions.

.markdownlint.json, line 4 at r2 (raw file):