dsccommunity / xRemoteDesktopSessionHost

This module contains DSC resources for the management and configuration of Microsoft Remote Desktop Session Host (RDSH).

MIT License

36 stars 47 forks source link

xRDSessionDeployment, xRDServer: Fixes for #47 and #50 #58

Closed danielboth closed 5 years ago

danielboth commented 5 years ago

Pull Request (PR) description

This PR contains the changes from PR #55 from @peppekerstens merged with dev. It fixes some typo's in parameters and a case where, when the RDMS service does not exist, the resource would throw an error.

This Pull Request (PR) fixes the following issues

Fixes #47
Fixes #50
Closes #55

Task list

[X] Added an entry under the Unreleased section of the change log in the README.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.
[X] 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

codecov-io commented 5 years ago

Codecov Report

Merging #58 into dev will increase coverage by 2.36%. The diff coverage is 65%.

@@            Coverage Diff            @@
##              dev     #58      +/-   ##
=========================================
+ Coverage   85.03%   87.4%   +2.36%     
=========================================
  Files           8       8              
  Lines         381     381              
  Branches        9       9              
=========================================
+ Hits          324     333       +9     
+ Misses         48      39       -9     
  Partials        9       9

Impacted Files	Coverage Δ
...RDSessionDeployment/MSFT_xRDSessionDeployment.psm1	`94.73% <100%> (ø)`	:arrow_up:
DSCResources/MSFT_xRDServer/MSFT_xRDServer.psm1	`66.12% <63.15%> (+14.51%)`	:arrow_up:

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 b2d1035...7f80885. Read the comment docs.

danielboth commented 5 years ago

Hi @gaelcolas, mind helping us out merging this one as well?

danielboth commented 5 years ago

More than a month ago. @ld0614 any chance you can look into this?

danielboth commented 5 years ago

Tagging @johlju for a review. Thanks in advance!

johlju commented 5 years ago

@danielboth or @gaelcolas could either of you add Closes #55 to the PR description so we close the other PR as well when this merges?

@gaelcolas can you please merge this PR.

gaelcolas commented 5 years ago

Thanks @danielboth & @johlju I'm just hilighting the Codecoverage diff hit in case you haven't seen it, but will merge anyway.

johlju commented 5 years ago

I ignored it since sometimes it gives false negatives. Code coverage report looked okay, and thought the changes made should be covered by existing tests.

Sure, we could have done TDD and make a (regression) test that fails before making the change, so it shows the change works. :)

johlju commented 5 years ago

After looking at the tests again it looks like there were tests made to cover this change. That’s why I ignored the code coverage status check (as it can give false negatives in some scenarios).