Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ScheduledTask: IdleWaitTimeout fix & BuiltInAccount added #187

Closed
wants to merge 43 commits into from
Closed

ScheduledTask: IdleWaitTimeout fix & BuiltInAccount added #187

wants to merge 43 commits into from

Conversation

djwork
Copy link
Contributor

@djwork djwork commented Oct 1, 2018

Pull Request (PR) description

ScheduledTask:
Fix to IdleWaitTimeout returned from Get-TargetResource always null
Added BuiltInAccount property to support running task as a builtin account

This Pull Request (PR) fixes the following issues

- Fixes #186 
- Fixes #130 

Task list

  • Added an entry under the Unreleased section of the change log in the CHANGELOG.md.
    Entry should say what was changed, and how that affects users (if applicable).
  • Resource documentation added/updated in README.md in the resource folder.
  • Resource parameter descriptions added/updated in 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 Reviewable

@PlagueHO
Copy link
Member

PlagueHO commented Oct 1, 2018

@djwork - looks like you've got some merge conflicts there - can you rebase against Dev?

@PlagueHO PlagueHO self-requested a review October 1, 2018 06:03
@PlagueHO PlagueHO added the waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. label Oct 1, 2018
@codecov-io
Copy link

codecov-io commented Oct 1, 2018

Codecov Report

Merging #187 into dev will decrease coverage by <1%.
The diff coverage is 73%.

Impacted file tree graph

@@        Coverage Diff         @@
##           dev   #187   +/-   ##
==================================
- Coverage   90%    89%   -1%     
==================================
  Files        7      7           
  Lines      724    758   +34     
==================================
+ Hits       657    681   +24     
- Misses      67     77   +10

@djwork
Copy link
Contributor Author

djwork commented Oct 1, 2018

@PlagueHO did my rebase come through? I am new to pull requests and rebasing

@djwork
Copy link
Contributor Author

djwork commented Oct 2, 2018

Is this still waiting on code fix?

@PlagueHO
Copy link
Member

PlagueHO commented Oct 2, 2018

Hi @djwork - sorry I haven't had a chance to look at this - will tonight. But in the mean time you've got some merge conflicts that need to be resolved. Let me know if you get stuck - always happy to help (and great job with the rebase - it is one of the harder techniques to master).

@djwork
Copy link
Contributor Author

djwork commented Oct 3, 2018

@PlagueHO fixed the merge conflicts

@PlagueHO
Copy link
Member

PlagueHO commented Oct 3, 2018

Hi @djwork - I think the rebase might have gone a bit wrong (I've been there more times than I can count 😁). Because you've not made a branch in your repo (just developed in dev), what you'd need to do is rebase against the upstream fork dev branch (I'm assuming you've registered a remote fork upstream with https://github.com/PowerShell/ComputerManagementDsc).

git rebase upstream dev

It should the replay all your commits onto the upstream dev branch. I generally create a branch of my own dev and then just pull all change from the upstream fork into my dev branch and do a squash rebase before I commit (to get rid of all my commits that say things like "This better work" and "another try" 😁)

@djwork
Copy link
Contributor Author

djwork commented Oct 3, 2018

I'm going to abandon this pull request and try again with a fresh one

@djwork djwork closed this Oct 3, 2018
@johlju johlju removed the waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. label Oct 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants