Skip to content

refactor: optimize ConvertTo-QueryString with better null handling #1026

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SoufiyanAk
Copy link
Contributor

  • Add null input validation to prevent errors
  • Improve null value handling for hashtable entries and object properties
  • Optimize performance by reducing redundant Get-Member calls
  • Enhance error messages with more context about supported types
  • Add proper ToString() conversion for non-null values

- Add null input validation to prevent errors
- Improve null value handling for hashtable entries and object properties
- Optimize performance by reducing redundant Get-Member calls
- Enhance error messages with more context about supported types
- Add proper ToString() conversion for non-null values
@SoufiyanAk SoufiyanAk requested a review from a team as a code owner July 14, 2025 07:35
@merill
Copy link
Contributor

merill commented Jul 21, 2025

@SoufiyanAk Since this is a core part of Maester I want to be extra careful that we don't cause any issues.

I've added some Pester tests in #1042

Would you be able to add additional tests for the changes you are making?

Can I know what prompted this PR?

Where there specific calls to Invoke-GraphRequest that were failing? If so, can you include them here in the comments and ideally as a pester test so we can verify the changes?

Also this test is doing a few different things in addition to the null check (eg. perf etc). Might be better to split them into different ones.

We are usually not this pedantic, but since this cmdlet is a core part of Maester and affects every test we need to be extra careful. Tx and much appreciated.

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.

2 participants