search

dsccommunity / UpdateServicesDsc

This module contains community maintained DSC resources for deployment and configuration of Windows Server Update Services.
MIT License
31 stars 27 forks source link

Fixing a few issues as per PowerShell/DscResources#346 #33

Closed gaelcolas closed 5 years ago

gaelcolas commented 5 years ago

Namely:

  1. README reference SqlServerDsc instead of UpdateServiceDsc
  2. Unit folder under Tests has a typo '.Unit' (starting with a '.')

I am also checking that appveyor builds this PR successfully.

Also, merging this to Dev will bring it up to date with master, and ahead with my commit.

Once merged to your Dev branch you should:

  1. Set Dev as your master branch (make sure you fork from there)
  2. Do a PR from Dev to Master and merge if it passes
gaelcolas commented 5 years ago

I've added more fixes but you still need to address 1, 3, 4, 6, 8, 12. What has been fixed:

  1. README reference SqlServerDsc instead of UpdateServiceDsc
  2. Unit folder under Tests has a typo '.Unit' (starting with a '.')
  3. CODE_OF_CONDUCT.md is missing but referenced in ReadMe.md
  4. Should include the metadata tag 'DSCResourceKit' in the module manifest (when approved).
  5. It seems that Markdown lint. Worked for me, but needs to see on AppVeyor (please enable AppVeyor for PR to master)
  6. Seems to be a few PSSA rules complaining in MSFT_UpdateServicesServer.psm1,MSFT_UpdateServicesCleanup.psm1 ('PSPossibleIncorrectComparisonWithNull' & Bracket style)
  7. had a compilation error of your test when I ran it locally (couldn't reproduce)
  8. You are missing a few elements from the DscResource.Template
  9. There's an Invoke-Expression in MSFT_UpdateServicesCleanup.psm1. PSSA Rules forbid it.

I've opted-in for more Tests (it might fail in AppVeyor)

PlagueHO commented 5 years ago

@gaelcolas - do you need a review on this one or are you taking care of that? Let me know if there is anything I can do to help (I've cleared some time for DSC this weekend so trying to cut through some reviews).

gaelcolas commented 5 years ago

I think the key element here is missing Integration tests & repo/appveyor/codecov config. I'll have another look when this is merged.

PlagueHO commented 5 years ago

If need be we can always get this in and then start submitting PR's to it to address issues one by one. Maybe @mgreenegit can make you a maintainer on this repo?