Reviewed 4 of 12 files at r1, 8 of 8 files at r2.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @PlagueHO)

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

  - Added `PartitionStyle` parameter - Fixes [Issue #137](https://github.com/PowerShell/StorageDsc/issues/37).
  - Changed MOF name from `MSFT_Disk` to `MSFTDSC_Disk` to remove conflict
    with Windows built-in CIM class.

Maybe reference issue here?

Modules/StorageDsc/DSCResources/MSFTDSC_Disk/MSFTDSC_Disk.psm1, line 42 at r2 (raw file):

This value of this parameter is ignored

This can be interpreted as it is used, but the value will be ignored. Maybe write 'This parameter is not used in Get-TargetResource'? Throughout.

Modules/StorageDsc/DSCResources/MSFTDSC_Disk/MSFTDSC_Disk.schema.mof, line 3 at r2 (raw file):

Prefix

maybe 'prefix' (lower-case 'p')?

Modules/StorageDsc/DSCResources/MSFTDSC_Disk/README.md, line 25 at r2 (raw file):

for disks with `GPT` partition table format.

What if the disk is RAW? Should the PartitionStyle be set to 'GPT' for the Guid identifier is allowed to be used?

Modules/StorageDsc/DSCResources/MSFTDSC_Disk/en-us/MSFTDSC_Disk.strings.psd1, line 21 at r2 (raw file):

'{3}' required.

maybe: '{3}' is required.

Modules/StorageDsc/DSCResources/MSFTDSC_Disk/en-us/MSFTDSC_Disk.strings.psd1, line 22 at r2 (raw file):

'{3}' required.

maybe: '{3}' is required.

Modules/StorageDsc/DSCResources/MSFTDSC_Disk/en-us/MSFTDSC_Disk.strings.psd1, line 22 at r2 (raw file):

'True'

Not sure, but maybe we should change this to $true, otherwise it could mean that the string 'True' should be used for a boolean value (for users less familiar with PowerShell)? Throughout, if so.

Tests/Integration/MSFTDSC_Disk.config.ps1, line 12 at r2 (raw file):

                DiskId          = $Node.DiskId
                DiskIdType      = $Node.DiskIdType
                PartitionStyle = $Node.PartitionStyle

Alignment wrong (after search/replace). 🙂

Tests/Integration/MSFTDSC_Disk.Integration.Tests.ps1, line 368 at r2 (raw file):

            }

            Context "Resize partition on Disk Number $($disk.Number) to use 50MB with AllowDestructive" {

Is this description correct? Doesn't it clear all partitions on the disk and converts it to GPT then creates a 50MB partition? Or do I misunderstood this test?

Should be good now @johlju

Reviewable status: 6 of 12 files reviewed, 9 unresolved discussions (waiting on @johlju and @PlagueHO)

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

Done.

Modules/StorageDsc/DSCResources/MSFTDSC_Disk/MSFTDSC_Disk.psm1, line 42 at r2 (raw file):

Done.

Modules/StorageDsc/DSCResources/MSFTDSC_Disk/MSFTDSC_Disk.schema.mof, line 3 at r2 (raw file):

Done. And changed the c in Class to lower case as it should be.

Modules/StorageDsc/DSCResources/MSFTDSC_Disk/README.md, line 25 at r2 (raw file):

The paragraph above specifies that a disk that is RAW can not be used with a GUID identifier or do you mean should we throw an exception if the MBR style is used and a Guid identifier type is specified?

Modules/StorageDsc/DSCResources/MSFTDSC_Disk/en-us/MSFTDSC_Disk.strings.psd1, line 21 at r2 (raw file):

Done.

Modules/StorageDsc/DSCResources/MSFTDSC_Disk/en-us/MSFTDSC_Disk.strings.psd1, line 22 at r2 (raw file):

Done.

Modules/StorageDsc/DSCResources/MSFTDSC_Disk/en-us/MSFTDSC_Disk.strings.psd1, line 22 at r2 (raw file):

Done.

Tests/Integration/MSFTDSC_Disk.config.ps1, line 12 at r2 (raw file):

Good catch! Done

Tests/Integration/MSFTDSC_Disk.Integration.Tests.ps1, line 368 at r2 (raw file):

You are correct. I also clarified another test that allowed destructive clearing of the disk.

Reviewed 6 of 6 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @johlju)

Modules/StorageDsc/DSCResources/MSFTDSC_Disk/README.md, line 25 at r2 (raw file):

I was just thinking if the disk is RAW and has no partition style yet (neither MBR or GPT), can I still specify 'Guid' as identifier type? I assume the disk can be neither GPT or MBR when it is newly created.
I just looking for if it should say: '...for disk with no partition format (RAW) or GPT partition table format'. 🙂

Tests/Integration/MSFTDSC_Disk.Integration.Tests.ps1, line 368 at r3 (raw file):

            }

            Context "Clear Disk Number $($disk.Number) and change the partition style to GPT and add a 50MB partition" {

Minor: Context should start with 'When...'. Throughout.

Leaving as non-blocking.

Should be good to go now @johlju! Thank you!

Reviewable status: 9 of 12 files reviewed, 1 unresolved discussion (waiting on @johlju and @PlagueHO)

Modules/StorageDsc/DSCResources/MSFTDSC_Disk/README.md, line 25 at r2 (raw file):

You are correct: If the disk is RAW then selecting GUID isn't possible. It will report back to the user that the disk can't be found.

I've reworked the text to hopefully make this a bit more clear.

Tests/Integration/MSFTDSC_Disk.Integration.Tests.ps1, line 368 at r3 (raw file):

Done! 😁 Lots!

Awesome work on this (as usual)! 😃

Reviewed 3 of 3 files at r4.
Reviewable status: complete! all files reviewed, all discussions resolved