search

lazywinadmin / AdsiPS

PowerShell module to interact with Active Directory using ADSI and the System.DirectoryServices namespace (.NET Framework)
http://www.lazywinadmin.com
MIT License
193 stars 46 forks source link

Bug Fix, Formatting Fix, Addition of Get-ADSIUserPrimaryGroup #74

Closed oze4 closed 5 years ago

oze4 commented 5 years ago

The following changes were made

@lazywinadmin - apologies for all of the pull requests lately! I wanted to make sure to get these fixed, though.

oze4 commented 5 years ago

@lazywinadmin I was able to get the issues resolved with Azure Pipeline tests.. All tests are now passing... With your approval, this Pull Request should be good to go - it resolves two open issues (#70 and #72)

lazywinadmin commented 5 years ago

Thanks @oze4 I'll review the code shortly, I fixed the issue yesterday but I left a remaining the Pester issue on purpose. I'm unsure why i see some of the changes i did yesterday in your PR. Thanks again

oze4 commented 5 years ago

@lazywinadmin The issue I found was that you cannot put any other links within the .NOTES section of any given comment block. More specifically, the .NOTES section has to contain the following URL, or else the build will fail... (this URL is required: https://github.com/lazywinadmin/ADSIPS) In my notes, I added the URL for each issue I was resolving, this caused the build to fail.

After I removed direct links to the issues within the .NOTES section, the build passed.

Screen Shot 2019-06-23 at 1 00 38 PM
oze4 commented 5 years ago

Thanks @oze4 I'll review the code shortly, I fixed the issue yesterday but I left a remaining the Pester issue on purpose. I'm unsure why i see some of the changes i did yesterday in your PR. Thanks again

Yea, that pull request was a little painful - that's my fault.

Most of the changes are there because I was trying to figure out why the build was failing. I have also attempted to address some of your other concerns above.