Skip to content
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

Added Windows "Client" OS support. Fixes #47 #48

Closed
wants to merge 2 commits into from

Conversation

mnothh
Copy link

@mnothh mnothh commented Jun 24, 2019

Pull Request (PR) description

Added support for Windows "Client" operating systems in the
Assert-HasPrereqsForBitlocker function.

This Pull Request (PR) fixes the following issues

-Fixes #47

Task list

  • Added an entry under the Unreleased section of the change log in the CHANGELOG.md.
    Entry should say what was changed, and how that affects users (if applicable).
  • Resource documentation added/updated in README.md.
  • Resource parameter descriptions added/updated in README.md, schema.mof
    and comment-based help.
  • Comment-based help added/updated.
  • Localization strings added/updated in all localization files as appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated. See DSC Resource Testing Guidelines.
  • Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
  • New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

This change is Reviewable

@msftclas
Copy link

msftclas commented Jun 24, 2019

CLA assistant check
All CLA requirements met.

@codecov-io
Copy link

Codecov Report

Merging #48 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@         Coverage Diff         @@
##            dev    #48   +/-   ##
===================================
  Coverage   100%   100%           
===================================
  Files         3      3           
  Lines        96     96           
===================================
  Hits         96     96

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ebb676e...1fb8d15. Read the comment docs.

@mnothh mnothh closed this Jun 24, 2019
Copy link
Contributor

@mhendric mhendric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r2.
Reviewable status: 1 of 3 files reviewed, 6 unresolved discussions (waiting on @mnothh)


Misc/xBitlockerCommon.psm1, line 797 at r2 (raw file):

            Write-Error 'The Bitlocker feature needs to be installed before the xBitlocker module can be used'

Can you use -Message named parameter per DSC style guidelines?


Misc/xBitlockerCommon.psm1, line 804 at r2 (raw file):

        $blFeature = Get-WindowsFeature BitLocker
        $blAdminToolsFeature = Get-WindowsFeature RSAT-Feature-Tools-BitLocker
        $blAdminToolsRemoteFeature = Get-WindowsFeature RSAT-Feature-Tools-BitLocker-RemoteAdminTool

Can you use named parameters here per DSC style guidelines? (I realize you didn't originally write this code, but it is getting modified here, so might as well fix it)


Misc/xBitlockerCommon.psm1, line 824 at r2 (raw file):

Write-Error 'The RSAT-Feature-Tools-BitLocker-RemoteAdminTool feature needs to be installed before the xBitlocker module can be used'

Can you use -Message named parameter per DSC style guidelines? (I realize you didn't originally write this code, but it is getting modified here, so might as well fix it)


Tests/Unit/xBitlockerCommon.tests.ps1, line 233 at r2 (raw file):

[string]

Object type should be properly cased ([String] or [System.String])


Tests/Unit/xBitlockerCommon.tests.ps1, line 282 at r2 (raw file):

[string]

Object type should be properly cased ([String] or [System.String])


Tests/Unit/xBitlockerCommon.tests.ps1, line 312 at r2 (raw file):

[string]

Object type should be properly cased ([String] or [System.String])

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resource do not work on Windows 10
4 participants