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

Added public function Assert-ElevatedUser #84

Merged
merged 5 commits into from
Dec 10, 2022

Conversation

hollanjs
Copy link
Contributor

@hollanjs hollanjs commented Dec 4, 2022

Pull Request (PR) description

Moved Assert-ElevatedUser from a private function in SqlServerDsc to a public function in DscResource.Common
Original issue logged in SqlServerDsc by @johlju Issue #1797

This Pull Request (PR) fixes the following issues

Task list

  • 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).
  • Documentation added/updated in README.md.
  • Comment-based help added/updated for all new/changed functions.
  • Localization strings added/updated in all localization files as appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated. See DSC Community Testing Guidelines.
  • Integration tests added/updated (where possible). See
    DSC Community Testing Guidelines.
  • New/changed code adheres to DSC Community Style Guidelines.

This change is Reviewable

@codecov
Copy link

codecov bot commented Dec 4, 2022

Codecov Report

Merging #84 (04e2d2c) into main (1228815) will increase coverage by 0%.
The diff coverage is 100%.

Impacted file tree graph

@@        Coverage Diff         @@
##           main   #84   +/-   ##
==================================
  Coverage    90%   90%           
==================================
  Files        27    28    +1     
  Lines       537   544    +7     
==================================
+ Hits        488   495    +7     
  Misses       49    49           
Impacted Files Coverage Δ
source/Public/Assert-ElevatedUser.ps1 100% <100%> (ø)

@hollanjs hollanjs closed this Dec 4, 2022
@hollanjs hollanjs reopened this Dec 4, 2022
@hollanjs
Copy link
Contributor Author

hollanjs commented Dec 4, 2022

@johlju - My PR associated with SqlServerDsc #1979, that moves the Assert-ElevatedUser to the DscResource.Common module. Please let me know if it requires any additional updates or changes.

Please disregard PR #83
Being my first PRs, I didn't realize all I had to do was update my branch. Before I knew this, I had already closed and opened a new one - lesson learned!

@johlju johlju added the needs review The pull request needs a code review. label Dec 5, 2022
@johlju
Copy link
Member

johlju commented Dec 5, 2022

@hollanjs no worries that you opened a new PR 🙂

Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3 of 4 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @hollanjs)

a discussion (no related file):
In the README.md we add the documentation for the command, like syntax. Please add the new command in the correct place (alphabetical order). Unfortunatly we have not yet fixed ths to update automatically, so I usually use the comment-based help and use Get-Help to get the syntax.


@johlju johlju added waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. and removed needs review The pull request needs a code review. labels Dec 5, 2022
@hollanjs
Copy link
Contributor Author

hollanjs commented Dec 7, 2022

@johlju - I added the cmdlet to the README.md; tried to be as detailed as possible but also succinct since the function really only does 1 thing and doesn't take inputs.

I also corrected the 'Syntax' block for Assert-IPAddress since it didn't follow the other subheading formats.

@johlju
Copy link
Member

johlju commented Dec 7, 2022

Great! I wil review this as soon as I can., at the latest on saturday.

@johlju johlju added needs review The pull request needs a code review. and removed waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. labels Dec 7, 2022
Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @hollanjs)

@johlju johlju merged commit 3fdb754 into dsccommunity:main Dec 10, 2022
@johlju
Copy link
Member

johlju commented Dec 10, 2022

Great work @hollanjs! I will do a full release of this module, then we can review and merge the PR in SqlServerDsc.

@johlju johlju removed the needs review The pull request needs a code review. label Dec 10, 2022
johlju pushed a commit that referenced this pull request Dec 10, 2022
- Add public function `Assert-ElevatedUser` that asserts the user has elevated
  the PowerShell session. issue #82
@hollanjs hollanjs deleted the fix-issue-82 branch December 16, 2022 23:51
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.

Add Assert-ElevatedUser from SqlServerDsc to DscResource.Common Public Functions
2 participants