Skip to content

Conversation

@indented-automation
Copy link
Contributor

PR Summary

Adds a module for ActiveDirectory with a short AboutFiltering topic.

Context

Attempts to cover frequently asked questions about filtering with the AD module.

Changes

Adds AboutFiltering.koans.ps1

Checklist

  • Pull Request has a meaningful title.
  • Summarised changes.
  • Pull Request is ready to merge & is not WIP.
  • Added tests / only testable interactively.
    • Make sure you add a new test if old tests do not effectively test the code changed.
  • Added documentation / opened issue to track adding documentation at a later date.

@vexx32 vexx32 added Category-Koans Invoking the Great Doubt PR-Needs-Review 🔍 Let's take a closer look! Module-ActiveDirectory Code or issues related to the Active Directory PowerShell module labels Jan 8, 2020
Copy link
Collaborator

@steviecoaster steviecoaster left a comment

Choose a reason for hiding this comment

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

LGTM, minus one small nitpick.

steviecoaster
steviecoaster previously approved these changes Jan 12, 2020
Copy link
Collaborator

@steviecoaster steviecoaster left a comment

Choose a reason for hiding this comment

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

Awesome!

Copy link
Owner

@vexx32 vexx32 left a comment

Choose a reason for hiding this comment

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

Found a few more bits and pieces that we should probably take care of before we merge. 🙂

vexx32
vexx32 previously approved these changes Jan 13, 2020
Copy link
Owner

@vexx32 vexx32 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

I have one follow-up question that's more of a general concern with this module. Since it relies on external data, these koans are going to be more fragile to work with by nature. I'm not sure it makes sense at this point, but perhaps in future it may make more sense to Mock the AD cmdlets rather than operate with live data?

@indented-automation
Copy link
Contributor Author

Yeah, I agree. Have to see which direction it goes in, we don't get all that many questions about things other than its complex filter parsing.

@vexx32 vexx32 removed the PR-Needs-Review 🔍 Let's take a closer look! label Jan 13, 2020
@vexx32 vexx32 merged commit 59fb800 into vexx32:master Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Category-Koans Invoking the Great Doubt Module-ActiveDirectory Code or issues related to the Active Directory PowerShell module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants