-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Evolve null-checking approach #26459
Conversation
@AndriySvyryd @smitpatel @maumar @bricelam @roji Sending this out with only changes to the Cosmos provider for now to make sure people get a chance to provide feedback before I do more. We discussed this a couple of years ago, so it would be good to validate that we still want to do this. |
@dotnet/efteam Today is your last chance to look at this before I merge and continue with more of the codebase. |
Part of #19233 Removes a lot of null checks from code that is not intended to be called directly by application code.
3377d77
to
6319926
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of the removals seem a bit user-facing to me; shouldn't we just remove null checks from private/internal/pubternal APIs?
Note that C# 10 almost got "simplified parameter null validation"; if this comes back in C# 11, the check would be as simple as a single bang after the parameter name (so less incentive to go remove it).
But am OK as-is as well.
@@ -37,7 +37,6 @@ public static class CosmosEntityTypeBuilderExtensions | |||
this EntityTypeBuilder entityTypeBuilder, | |||
string? name) | |||
{ | |||
Check.NotNull(entityTypeBuilder, nameof(entityTypeBuilder)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this a pretty user-facing API? Or did we say we don't care about checking null "this" for extension methods?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this for this.
Part of #19233
Removes a lot of null checks from code that is not intended to be called directly by application code.