Submit UpdateServicesDsc for DscResources repo listing

[Michael Greene (mgreenegit)]

DSC Community -

I would like to request for this DSC module, UpdateServicesDsc, to be considered for listing as a submodule in the DSCResources repo.

Closes #187

---

This change is 

[Daniel Scott-Raynsford (PlagueHO)]

Is this going to be the method going forward we should use for other community maintained DSC Resource modules? I have some modules I wanted to get linked to as well.

[Johan Ljunggren (johlju)]

Michael Greene (@mgreenegit) A few comments about the UpdateServicesDsc. These are pointed out as of the https://github.com/PowerShell/DscResources/blob/master/Naming.md#quality-governance. Maybe some of these are up to the maintainer. Maybe some of these can be resolved once the repository is added to the DSC Resource Kit.
This is a the first one, so this set precedence. 😄

I did a really quick review of the UpdateServicesDsc repo. This is my thoughts, up for discussion.

1. It should have a dev branch, and dev branch should be the default branch for new PR's.
2. It has two branches that might be unnecessary. This is probably up to the maintainer.
3. It doesn't seem connected to AppVeyor so new merges (and PR's) does not get tested (see commits https://github.com/mgreenegit/UpdateServicesDsc/commits/master)
4. README.md referencing naming conventions including the use of 'c' suffix. This should be updated/removed.
5. Doesn't use the normal template in the appveyor.yml - not sure if we have an official , but we should. Created a gist containing a template that could be merged as a template to DscResources; Template appveyor.yml for default testing model for the DscResource.Tests framework. Now the default template of appveyor.yml can be found in DscResources.
6. It should have Codecov added, and preferably README.md updated with badges (see example in README.md).
7. It should have either a CONTRIBUTING.md file or a 'Contributing' section in the README.md file.
8. Unit folder under Tests has a typo '.Unit' (starting with a '.'). Might be a problem when tests are run in AppVeyor using test framework.
9. Unit tests should updated to using the latest unit test template from DscResources repository.
10. Integration test does not actually test if the values were set.
11. Should have either a CODE_OF_CONDUCT.md or a 'Code of conduct' section in the README.md. Since I think we cannot use Microsoft's code of conduct for these repos, GitHub can help create a CODE_OF_CONDUCT.md file in the repos Settings/Insight/Community.
12. Add the repository to the list in the Maintainers.md file in DscResources repo.
13. Should include the metadata tag 'DSCResource' in the module manifest.
14. Should include the metadata tag 'DSCResourceKit' in the module manifest (when approved). Not sure about this one.

Update: Updated to use numbers instead of bullets, for easier reference.
Update 2: Added code of conduct entry.
Update 3: Missed that README.md contained a Contributing section at the bottom.
Update 4: The default template of appveyor.yml was merged to DscResource. Updated the list with the new URL.
Update 5: Added Maintainers.md to the list of steps.
Update 6: Added a note about the metadata tag should be included in the module manifest, which it already is, but added it as an reminder for next resource module submission.
Update 7: Added a note about the metadata tag 'DSCResourceKit'.

[Daniel Scott-Raynsford (PlagueHO)]

Great list Johan Ljunggren (@johlju) - Michael Greene (@mgreenegit) - would you be up for accepting PRs to your repo so we can tweak these things?

[Johan Ljunggren (johlju)]

Should we update the repo list in the Maintainers.md file as well for these new modules that get added?

[Michael Greene (mgreenegit)]

Good point. I hope to get back to this today.

[stale]

Labeling this pull request (PR) as abandoned since it has gone 14 days or more since the last update. An abandoned PR can be continued by another contributor. The abandoned label will be removed if work on this PR is taken up again.

[Johan Ljunggren (johlju)]

This was wrongly closed by Stale bot

[stale]

Labeling this pull request (PR) as abandoned since it has gone 14 days or more since the last update. An abandoned PR can be continued by another contributor. The abandoned label will be removed if work on this PR is taken up again.

[stale]

Labeling this pull request (PR) as abandoned since it has gone 14 days or more since the last update. An abandoned PR can be continued by another contributor. The abandoned label will be removed if work on this PR is taken up again.

[Michael Greene (mgreenegit)]

Daniel Scott-Raynsford (@PlagueHO) Johan Ljunggren (@johlju) gaelcolas (@gaelcolas)
Just FYI, I made a new round of updates this morning to try and bring this back for consideration in the resource kit.

[Michael Greene (mgreenegit)]

since the original repo I submitted this from is long gone, I might have to restart the PR to resolve conflicts? I just wanted to get feedback first.

[gaelcolas (gaelcolas)]

Having a look.
There's a few things I can see, will update with details shortly.

[gaelcolas (gaelcolas)]

Here's what I found so far.

1. New PR would be nice
2. README reference SqlServerDsc instead of UpdateServiceDsc
3. Your default branch (configured in GH) is master, please change to Dev
4. You seem to have CodeCov only configured for the master branch (not Dev)
5. Unit folder under Tests has a typo '.Unit' (starting with a '.')
6. Integration test does not actually test if the values were set.
7. CODE_OF_CONDUCT.md is missing but referenced in ReadMe.md
8. Add the repository to the list in the Maintainers.md file in DscResources repo (when doing new PR)
9. Should include the metadata tag 'DSCResourceKit' in the module manifest (when approved). Not sure about this one (Johan Ljunggren (@johlju), Daniel Scott-Raynsford (@PlagueHO) ?)
10. It seems that Markdown lint failed on AppVeyor but should be skipped, will investigate
11. Seems to be a few PSSA rules complaining in MSFT_UpdateServicesServer.psm1,MSFT_UpdateServicesCleanup.psm1 ('PSPossibleIncorrectComparisonWithNull' & Bracket style)
12. Missing Examples (but optional)
13. had a compilation error of your test when I ran it locally, will have a look

"The property 'WorkingFolder' cannot be found on this object. Verify that the property exists." was thrown from C:\src\UpdateServicesDsc\Tests\Integration\UpdateServicesDsc.tests.ps1:28 char:13

15. You are missing a few elements from the DscResource.Template

• .vscode folder & config
• .github folder & templates
• Changelog
• code of conduct
• Examples

16. There's an Invoke-Expression in MSFT_UpdateServicesCleanup.psm1. PSSA Rules forbid it.

As it's a new module that aims at being an HQRM, you could opt-in to more test (it would have caught 8).

[
    "Common Tests - Validate Markdown Files",
    "Common Tests - Validate Example Files",
    "Common Tests - Validate Module Files",
    "Common Tests - Validate Script Files",
    "Common Tests - Required Script Analyzer Rules",
    "Common Tests - Flagged Script Analyzer Rules",
    "Common Tests - New Error-Level Script Analyzer Rules",
    "Common Tests - Custom Script Analyzer Rules"
]

[gaelcolas (gaelcolas)]

And your DEV branch is way behind Master by 13 commits.
Work should be done on Dev and 'released' to master as per current workflow.

Ryan Bartram (@rdbartram) might help you out with the issues above? :)

I've also opened a quick PR.

[Daniel Scott-Raynsford (PlagueHO)]

Awesome Michael Greene (@mgreenegit) and gaelcolas (@gaelcolas) - can't wait to get this one in!

Regards 9: From memory (Johan Ljunggren (@johlju) might correct this) but we did agree that we'd add the DSCResourceKit tag for any community module that was listed here (and therefore met the HQRM standards).

[Johan Ljunggren (johlju)]

The 'New resource module submission' issue template contains the below.

It just says tag 'DSCResource', but this is just a template, so we can add the tag 'DscResourceKit' as well. I think that would be good because that would mean it belongs to the "family" of the other modules.

The new resource module:

• is licensed under MIT.
• has the correct project structure.
• has a dev and master branch, and dev should default branch.
• is using DSC Resource Kit test templates.
• is connected to Codecov or similar.
• has established a code of conduct in ether a CODE_OF_CONDUCT.md or a
  'Code of conduct' section in the README.md.
• has either a CONTRIBUTING.md file or a 'Contributing' section in the
  README.md file.
• has included the metadata tag 'DSCResource' in the module manifest.
• is sufficiently documented.
• has an established CI process for each push to the repository.
• is sufficiently tested.
• Follows the guidelines set out for a
  High Quality Resource Module.

[gaelcolas (gaelcolas)]

I fixed a bunch of other things in the open PR: dsccommunity/UpdateServicesDsc#33
You still need to address:
1, 3, 4, 6, 8, 12

1. New PR would be nice
2. Your default branch (configured in GH) is master, please change to Dev
3. You seem to have CodeCov only configured for the master branch (not Dev)
4. Integration test does not actually test if the values were set.
5. Add the repository to the list in the Maintainers.md file in DscResources repo (when doing new PR)
6. Missing Examples (but optional)

Adding another one:
17. It seems that your AppVeyor is not configured to build PRs (to Dev). So I can't test my PR builds successfully on AppVeyor

[gaelcolas (gaelcolas)]

The PR has been merged and the repo configured.
It is missing Integration Tests, but that probably should not be a blocker, as we can help improve the resource over time.
I think we're ready to include that one with the DSC Resource Kit.

Johan Ljunggren (@johlju) & Daniel Scott-Raynsford (@PlagueHO): What do you think? Anything I'm missing?

[Johan Ljunggren (johlju)]

You don’t think we should enforce having integration tests where possible? If not then we can allow other resources to be added without integration tests. If so we should update the module submission issue template (and documentation) that it is not necessary to be “HQRM”?

If so I’m okay with this.

[gaelcolas (gaelcolas)]

It's a balance of:

• ensuring continued quality and trust in the resource kit
• having easy to run Integration tests (see NetAdapterState: New resource to enable or disable a network adapter dsccommunity/NetworkingDsc#379)
• giving a chance to newcomers, and support their growth in the community by being inclusive (not leaving them at the door for months)

I've put the thinking in #490 where we can have the generic discussion about how to make such choice. Then we can come back and make one for this specific case.

[gaelcolas (gaelcolas)]

FTR a new version has been released to the gallery, and I've implemented some integration tests using Test-Kitchen.
I'm now happy with the quality, although more integration tests could be written.
Given the changes going on, I'm going to put this one on hold and we'll revisit in a month or so.

[gaelcolas (gaelcolas)]

As announced during the last community call, we'd love to have this as part of dsccommunity. I'll work to transfer this one over along with the others.

[Michael Greene (mgreenegit)]

https://github.com/dsccommunity/UpdateServicesDsc