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

Port #190 from 2.4 #206

Merged
merged 1 commit into from
Nov 16, 2024
Merged

Port #190 from 2.4 #206

merged 1 commit into from
Nov 16, 2024

Conversation

jeremy-visionaid
Copy link
Collaborator

No description provided.

@Numpsy
Copy link
Contributor

Numpsy commented Nov 15, 2024

I'll give this a test after lunch


// As per https://learn.microsoft.com/en-us/openspecs/windows_protocols/MS-OLEPS/4177a4bc-5547-49fe-a4d9-4767350fd9cf
// the property names have to be unique, and are case insensitive.
if (PropertyNames.Any(property => property.Value.Equals(name, StringComparison.InvariantCultureIgnoreCase)))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is causing a 'PropertyNames might be null' warning, though it should never be null in a UserDefinedProperties container since #117

Copy link
Collaborator Author

@jeremy-visionaid jeremy-visionaid Nov 16, 2024

Choose a reason for hiding this comment

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

@Numpsy Thanks for the review! I enabled nullability warnings for 3.0. The warnings here look legitimate, since PropertyNames isn't initialized in the constructor. I did the least amount of work to satisfy the compiler - i.e. I added the missing attributes rather than fixed the initialization. If you want to fix up the nullability attributes/initialization accordingly in a later commit, then that would be great. You'd know much more about the expectations about whether something should/shouldn't be allowed to be null - I've merely enabled the enforcement :)

@jeremy-visionaid jeremy-visionaid marked this pull request as ready for review November 16, 2024 08:31
@jeremy-visionaid jeremy-visionaid merged commit 162d227 into master Nov 16, 2024
2 checks passed
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