-
Notifications
You must be signed in to change notification settings - Fork 141
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
xActiveDirectory: Use fully qualified type names #385
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #385 +/- ##
===================================
Coverage 92% 92%
===================================
Files 20 20
Lines 2530 2530
Branches 10 10
===================================
Hits 2335 2335
Misses 185 185
Partials 10 10 |
@PlagueHO Could you review this when you have time. A lot of files, but I think this one will be easy reviewed anyway. 🙂 |
Of course @johlju - I'm on it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of minor tweaks. But awesome job!
Reviewed 43 of 44 files at r1, 2 of 2 files at r2.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @johlju)
Examples/Resources/xADUser/1-CreateUserAndManagePassword_Config.ps1, line 43 at r2 (raw file):
{ Ensure = 'Present' UserName = "ExampleUser"
Strings should be single quoted (and next few).
Examples/Resources/xADUser/2-CreateUserAndIgnorePasswordChanges_Config.ps1, line 44 at r2 (raw file):
{ Ensure = 'Present' UserName = "ExampleUser"
Strings should be single quoted (and next few).
Tests/Unit/MSFT_xADGroup.Tests.ps1, line 450 at r2 (raw file):
Mock -CommandName Get-DomainName -MockWith {return 'contoso.com'} Mock -CommandName Get-ADDomainNameFromDistinguishedName -MockWith { param (
( should be on next line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I formatted the parameters of some stub functions. I did not add [Parameter()]
to them on purpose, I would rather we solved having a module with stubs so they could be removed entirely.
Reviewable status: 40 of 50 files reviewed, 3 unresolved discussions (waiting on @PlagueHO)
Examples/Resources/xADUser/1-CreateUserAndManagePassword_Config.ps1, line 43 at r2 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
Strings should be single quoted (and next few).
Done.
Examples/Resources/xADUser/2-CreateUserAndIgnorePasswordChanges_Config.ps1, line 44 at r2 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
Strings should be single quoted (and next few).
Done.
Tests/Unit/MSFT_xADGroup.Tests.ps1, line 450 at r2 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
( should be on next line
Done. Fix all in the repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 10 of 10 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved
@PlagueHO Thanks for the review! This one is starting to get closer and closer for a rename. |
Pull Request (PR) description
(issue xActiveDirectory: Use fully qualified type names for parameters and variables #374).
Also updated the examples a bit, that should already be covered by another change log entry.
This Pull Request (PR) fixes the following issues
Task list
Entry should say what was changed, and how that affects users (if applicable).
and comment-based help.
This change is