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

Add separate check for strings #5463

Open
wants to merge 5 commits into
base: Dev
Choose a base branch
from

Conversation

FabienTschanz
Copy link
Contributor

@FabienTschanz FabienTschanz commented Nov 25, 2024

Pull Request (PR) description

This PR improves the comparison of strings inside of Compare-M365DSCComplexObject. Previously, the comparison did not check for different line breaks (\r\n vs \n). The comparison was also updated to use StringComparison.Ordinal, a byte-wise check which does not take into consideration the current culture.

This Pull Request (PR) fixes the following issues

None.

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).
  • Resource parameter descriptions added/updated in the schema.mof.
  • Resource documentation added/updated in README.md.
  • Resource settings.json file contains all required permissions.
  • Examples appropriately added/updated.
  • Unit tests added/updated.
  • New/changed code adheres to DSC Community Style Guidelines.

@ricmestre
Copy link
Contributor

ricmestre commented Nov 25, 2024

Nitpicking, but comment # Remove carriage return and line feed characters is incorrect, you're replacing CRLF characters with LF so you could say you're only removing carriage return. Also not that it makes a difference but everything in M365DSC is done with CRLF in mind but you're actually replacing it with LF, again not that it makes a difference in the end result but just saying.

Also, this is something that is missing from plain string comparisons not inside CIM instances, I have a monitoring tool which I have to massage the results with something very similar so that they're not reported as drifts, can't just remember which function it is but it should be in M365DSCUtil.psm1

@FabienTschanz
Copy link
Contributor Author

@ricmestre Will update my comment 👀 Is the function you mean Test-M365DSCParameterState?

@FabienTschanz FabienTschanz force-pushed the feat/string-comparison-improvements branch from 4d3ca3b to b06cb2e Compare November 25, 2024 22:51
@FabienTschanz FabienTschanz force-pushed the feat/string-comparison-improvements branch from b06cb2e to 4065266 Compare November 25, 2024 23:20
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