ComputerManagementDsc icon indicating copy to clipboard operation
ComputerManagementDsc copied to clipboard

Fix mismatch between user and builtinaccount

Open nickgw opened this issue 2 years ago • 6 comments

Currently ScheduledTask conflates the param BuiltInAccount and *-ScheduledTask cmdlet's user parameter. This fixes that.

Pull Request (PR) description

Currently ScheduledTask conflates the param BuiltInAccount and *-ScheduledTask cmdlet's user parameter. This fixes that.

This Pull Request (PR) fixes the following issues

  • Fixes #385

Task list

  • [x] Added an entry to the change log under the Unreleased section of the file CHANGELOG.md. Entry should say what was changed and how that affects users (if applicable), and reference the issue being resolved (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 Community Testing Guidelines.
  • [ ] Integration tests added/updated (where possible). See DSC Community Testing Guidelines.
  • [x] New/changed code adheres to DSC Community Style Guidelines.

This change is Reviewable

nickgw avatar May 21 '22 21:05 nickgw

Codecov Report

Merging #387 (73dce2d) into main (3aea926) will increase coverage by 0%. The diff coverage is 100%.

Impacted file tree graph

@@         Coverage Diff         @@
##           main   #387   +/-   ##
===================================
  Coverage    90%    90%           
===================================
  Files        17     17           
  Lines      1716   1726   +10     
===================================
+ Hits       1554   1564   +10     
  Misses      162    162           
Impacted Files Coverage Δ
...Resources/DSC_ScheduledTask/DSC_ScheduledTask.psm1 87% <100%> (+<1%) :arrow_up:

codecov[bot] avatar Jun 03 '22 14:06 codecov[bot]

@PlagueHO This should be good to go now, when you get a chance to check it out!

nickgw avatar Jun 03 '22 14:06 nickgw

Should the MOF schema be updated to match this as well?

https://github.com/dsccommunity/ComputerManagementDsc/blob/main/source/DSCResources/DSC_ScheduledTask/DSC_ScheduledTask.schema.mof#L16

ShawnHardwick avatar Jun 20 '22 20:06 ShawnHardwick

Should the MOF schema be updated to match this as well?

https://github.com/dsccommunity/ComputerManagementDsc/blob/main/source/DSCResources/DSC_ScheduledTask/DSC_ScheduledTask.schema.mof#L16

The inputs don't actually change. The code was comparing the wrong properties.

nickgw avatar Jun 20 '22 21:06 nickgw

Hi @nickgw - sorry was tied up on other things. I'll get onto this close to the end of the week. Just need to try and avoid breaking Windows Server 2016 (unfortunately we can't test on it at the moment unless we get something like TK going in Azure).

PlagueHO avatar Jun 21 '22 07:06 PlagueHO

Hey @PlagueHO just bumping this (and my related comments in the issue) when you get a chance!

nickgw avatar Jul 08 '22 21:07 nickgw

@PlagueHO Do you mind taking another look at this? I've added another test.

nickgw avatar Oct 29 '22 19:10 nickgw

@PlagueHO do you mind taking another look at this? I'd love to close this off my list.

nickgw avatar Nov 19 '22 22:11 nickgw

Cool! Thanks @nickgw - I'll review tonight.

PlagueHO avatar Nov 20 '22 21:11 PlagueHO

@PlagueHO I think I hit all your comments!

nickgw avatar Nov 22 '22 16:11 nickgw