-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Use ArgumentException.ThrowIfNullOrEmpty part 1
#19215
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
Use ArgumentException.ThrowIfNullOrEmpty part 1
#19215
Conversation
ArgumentNullException.ThrowIfNull part 1ArgumentException.ThrowIfNullOrEmpty part 1
|
@xtqqczze The change is for adopting |
This comment was marked as resolved.
This comment was marked as resolved.
Although I've mixed the following changes when they could have been seperated.:
In any case the changes are to non-public APIS. |
| } | ||
|
|
||
| ArgumentException.ThrowIfNullOrEmpty(optionName); | ||
|
|
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.
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.
My personal preference is to separate the method calls with a blank line when the type of the ArgumentException differs.
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.
I like it, I'll update my PR following your style 👍
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.
I think it's better to group them together as they are doing the same kind of things. For example, we group Dbg.Assert(xxx) together at the beginning of some methods, even though they do different checks.
daxian-dbw
left a comment
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.
LGTM
|
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
Use
ArgumentException.ThrowIfNullOrEmptyin Microsoft.PowerShell.Commands.Management.Contributes to #19212.
Changes
ArgumentExceptioninstead ofArgumentNullExceptionwhen a string is empty.ArgumentExceptionis a less derived type.ArgumentNullExceptioninstead ofArgumentExceptionwhen a string is null.ArgumentNullExceptionis a more derived type.Breaking changes contract
Bucket 4: Clearly Non-Public
Microsoft.PowerShell.Cmdletization.Cim.CimQueryMicrosoft.PowerShell.Commands.CIMHelperOther changes
608186d is a related refactoring that does not change behavior.