Correct Style Guideline violation in xGroup - Fixes #485 #508

PlagueHO · 2019-01-28T02:54:07Z

Pull Request (PR) description

This PR correct style guideline violations in xGroup. Trailing white space was also trimmed automatically by VSCode.

This Pull Request (PR) fixes the following issues

Fixes MSFT_xGroupResource.psm1: Fix Script Analyzer Issues #485

Task list

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.
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 · 2019-01-28T03:34:50Z

Codecov Report

Merging #508 into dev will not change coverage.
The diff coverage is 66%.

@@         Coverage Diff         @@
##            dev   #508   +/-   ##
===================================
  Coverage    74%    74%           
===================================
  Files        27     27           
  Lines      4031   4031           
  Branches      4      4           
===================================
  Hits       3005   3005           
  Misses     1022   1022           
  Partials      4      4

mhendric

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @PlagueHO)

a discussion (no related file):
Hey @PlagueHO. All the changes look good. However it looks like there's still one more Script Analyzer warning we can address (although it doesn't show up in the CI report). Would you be able to get this one while you're at it? Thanks.

PS D:\xPSDesiredStateConfiguration-Issue-485\xPSDesiredStateConfiguration-Issue-485\DSCResources\MSFT_xGroupResource> in
voke-scriptanalyzer .\MSFT_xGroupResource.psm1

RuleName                            Severity     ScriptName Line  Message
--------                            --------     ---------- ----  -------
PSUseSingularNouns                  Warning      MSFT_xGrou 2473  The cmdlet 'Clear-GroupMembers' uses a plural noun.
                                                 pResource.       A singular noun should be used instead.
                                                 psm1

PlagueHO · 2019-01-30T08:53:49Z

Thanks @mhendric - sorry for not getting anything done this week. Still traveling for work. Will get back into this over the weekend.

PlagueHO

Reviewable status: 1 of 3 files reviewed, 1 unresolved discussion (waiting on @mhendric)

a discussion (no related file):

Previously, mhendric (Mike Hendrickson) wrote…

Hey @PlagueHO. All the changes look good. However it looks like there's still one more Script Analyzer warning we can address (although it doesn't show up in the CI report). Would you be able to get this one while you're at it? Thanks.

PS D:\xPSDesiredStateConfiguration-Issue-485\xPSDesiredStateConfiguration-Issue-485\DSCResources\MSFT_xGroupResource> in
voke-scriptanalyzer .\MSFT_xGroupResource.psm1

RuleName                            Severity     ScriptName Line  Message
--------                            --------     ---------- ----  -------
PSUseSingularNouns                  Warning      MSFT_xGrou 2473  The cmdlet 'Clear-GroupMembers' uses a plural noun.
                                                 pResource.       A singular noun should be used instead.
                                                 psm1

Doh! Good catch. Fixed. I'll also fix in PSDscResources (it is a problem there too).

mhendric · 2019-02-02T16:39:16Z

Arggg… #505 strikes again. Looks like this one will need to have CI kicked off again.

43
Executing script C:\projects\xpsdesiredstateconfiguration\Tests\Integration\xWindowsOptionalFeatureSet.Integration.Tests.ps1
5244
  [-] Error occurred in test script 'C:\projects\xpsdesiredstateconfiguration\Tests\Integration\xWindowsOptionalFeatureSet.Integration.Tests.ps1' 0ms
5245
    RemoteException: fatal: unable to access 'https://github.com/PowerShell/DscResource.Tests/': Could not resolve host: github.com
5246
    at Enter-DscResourceTestEnvironment, C:\projects\xpsdesiredstateconfiguration\Tests\CommonTestHelper.psm1: line 748
5247
    at <ScriptBlock>, C:\projects\xpsdesiredstateconfiguration\Tests\Integration\xWindowsOptionalFeatureSet.Integration.Tests.ps1: line 9
5248
    at <ScriptBlock>, C:\Program Files\WindowsPowerShell\Modules\Pester\4.6.0\Pester.psm1: line 1095
5249
    at Invoke-Pester<End>, C:\Program Files\WindowsPowerShell\Modules\Pester\4.6.0\Pester.psm1: line 1121
5250
    at Invoke-AppveyorTestScriptTask, C:\projects\xpsdesiredstateconfiguration\DscResource.Tests\AppVeyor.psm1: line 792
5251
5252
Executing script C:\projects\xpsdesiredstateconfiguration\Tests\Unit\CommonResourceHelper.Tests.ps1
5253

mhendric

Reviewed 2 of 2 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved

PlagueHO · 2019-02-02T20:24:00Z

Sorry @mhendric - can you review again - I had to rebase after the last merge. Should be trivial 🤞

mhendric

Reviewed 1 of 2 files at r3.
Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @mhendric and @PlagueHO)

Tests/Unit/MSFT_xGroupResource.Tests.ps1, line 932 at r3 (raw file):

Quoted 5 lines of code…

<<<<<<< HEAD
                    Mock -CommandName 'Clear-GroupMembers' -MockWith { }
=======
                    Mock -CommandName 'Clear-GroupMember' -MockWith { }
>>>>>>> Renamed Clear-GroupMembers helper function to Clear-GroupMember to remove violation of PowerShell best practice.

Looks like a failed merge needs to be corrected.

PlagueHO

Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @mhendric)

Tests/Unit/MSFT_xGroupResource.Tests.ps1, line 932 at r3 (raw file):

Previously, mhendric (Mike Hendrickson) wrote…

<<<<<<< HEAD
                    Mock -CommandName 'Clear-GroupMembers' -MockWith { }
=======
                    Mock -CommandName 'Clear-GroupMember' -MockWith { }
>>>>>>> Renamed Clear-GroupMembers helper function to Clear-GroupMember to remove violation of PowerShell best practice.

Looks like a failed merge needs to be corrected.

Done.

PlagueHO

This time hopefully! Thanks @mhendric!

Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @mhendric)

…move violation of PowerShell best practice.

PlagueHO · 2019-02-02T22:12:30Z

Rebased now and moved change log to new CHANGELOG.MD - should be good to go now @mhendric

mhendric

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

PlagueHO added the needs review The pull request needs a code review. label Jan 28, 2019

PlagueHO requested a review from mhendric January 28, 2019 02:54

mhendric requested changes Jan 28, 2019

View reviewed changes

mhendric added waiting for author response The pull request is waiting for the author to respond to comments in the pull request. and removed needs review The pull request needs a code review. labels Jan 28, 2019

PlagueHO commented Feb 2, 2019

View reviewed changes

mhendric approved these changes Feb 2, 2019

View reviewed changes

PlagueHO force-pushed the Issue-485 branch from a59c671 to f9ba07a Compare February 2, 2019 20:22

PlagueHO added needs review The pull request needs a code review. and removed waiting for author response The pull request is waiting for the author to respond to comments in the pull request. labels Feb 2, 2019

mhendric requested changes Feb 2, 2019

View reviewed changes

PlagueHO commented Feb 2, 2019

View reviewed changes

PlagueHO added 3 commits February 3, 2019 11:08

Correct Style Guideline violation in xGroup

c02e097

Renamed Clear-GroupMembers helper function to Clear-GroupMember to re…

7079313

…move violation of PowerShell best practice.

Clean up uneccessary line in README.MD

eacc381

PlagueHO force-pushed the Issue-485 branch from 914518c to eacc381 Compare February 2, 2019 22:11

mhendric approved these changes Feb 3, 2019

View reviewed changes

mhendric merged commit b27ceb6 into dsccommunity:dev Feb 3, 2019

PlagueHO deleted the Issue-485 branch February 3, 2019 02:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correct Style Guideline violation in xGroup - Fixes #485 #508

Correct Style Guideline violation in xGroup - Fixes #485 #508

PlagueHO commented Jan 28, 2019 •

edited by joeyaiello

Loading

codecov-io commented Jan 28, 2019 •

edited

Loading

mhendric left a comment

PlagueHO commented Jan 30, 2019

PlagueHO left a comment

mhendric commented Feb 2, 2019

mhendric left a comment

PlagueHO commented Feb 2, 2019

mhendric left a comment

PlagueHO left a comment

PlagueHO left a comment

PlagueHO commented Feb 2, 2019

mhendric left a comment

Correct Style Guideline violation in xGroup - Fixes #485 #508

Correct Style Guideline violation in xGroup - Fixes #485 #508

Conversation

PlagueHO commented Jan 28, 2019 • edited by joeyaiello Loading

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

Task list

codecov-io commented Jan 28, 2019 • edited Loading

Codecov Report

mhendric left a comment

Choose a reason for hiding this comment

PlagueHO commented Jan 30, 2019

PlagueHO left a comment

Choose a reason for hiding this comment

mhendric commented Feb 2, 2019

mhendric left a comment

Choose a reason for hiding this comment

PlagueHO commented Feb 2, 2019

mhendric left a comment

Choose a reason for hiding this comment

PlagueHO left a comment

Choose a reason for hiding this comment

PlagueHO left a comment

Choose a reason for hiding this comment

PlagueHO commented Feb 2, 2019

mhendric left a comment

Choose a reason for hiding this comment

PlagueHO commented Jan 28, 2019 •

edited by joeyaiello

Loading

codecov-io commented Jan 28, 2019 •

edited

Loading