-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Enable nullable: System.Management.Automation.ICommandRuntime #14189
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
Conversation
I suspect that we ideally would not, but I'm not sure if making such a change would break anything |
|
IParameterMetadataProvider has 5 dependencies. This complicates understanding design intentions and we will review this later on phase 5. |
Nullable annotations can not break anything. They only explicitly expose design intentions. |
|
This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 15 days. It will be closed if no further activity occurs within 10 days of this comment. |
This comment has been minimized.
This comment has been minimized.
|
Hmm, we never check for null in the WriteDebug() API until low level host implementations where we ignore null messages. |
|
This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 15 days. It will be closed if no further activity occurs within 10 days of this comment. |
|
WriteObject makes sense. The throws make sense. The skips trouble me, since changing them is breaking, but it would be nice to change the type signature. But I'm not sure I see the value there. I guess the best course of action is to label the APIs as you've researched them @iSazonov |
Nullable annotations are only about design intentions. So we can annotate all the "skips" as non-nullable without adding real null checks and de-facto without breaking changes. |
Yeah that's perfect |
|
The PR branch was removed so I opened new PR #15566. |
Tracking issue: #12631.
Should we allow null for the parameters to
WriteDebug, WriteWarning, WriteVerbose, etc?