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

Remove ValidationException #749

Merged
merged 2 commits into from
May 1, 2023
Merged

Conversation

stephentoub
Copy link
Member

Motivation and Context

Remove custom ValidationException in favor of using built-in .NET exception types typically used for the same purposes.

Description

The conditions ValidationException was used for validating are better suited to existing .NET exception types that .NET developers expect in those situations.

  • NotNullOrWhiteSpace is represented with ArgumentException.
  • Directory not found is represented with DirectoryNotFoundException
  • Invalid state of an object (e.g. a field not being initialized) is represented with InvalidOperationException

Argument validation exceptions are also usage exceptions typically not intended to be caught. The same would go for the ValidationExceptions that were serving the same purpose, so having a separate type identity for those exceptions isn't desirable.

Contribution Checklist

cc: @shawncal, @dluc

Sorry, something went wrong.

@github-actions github-actions bot added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel kernel.core labels May 1, 2023
@stephentoub stephentoub force-pushed the validationexception branch from 4be2266 to 83ba112 Compare May 1, 2023 16:40

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
The conditions ValidationException was used for validating are better suited to existing .NET exception types that .NET developers expect in those situations.

- NotNullOrWhiteSpace is represented with ArgumentException.
- Directory not found is represented with DirectoryNotFoundException
- Invalid state of an object (e.g. a field not being initialized) is represented with InvalidOperationException

Argument validation exceptions are also usage exceptions typically not intended to be caught.  The same would go for the ValidationExceptions that were serving the same purpose, so having a separate type identity for those exceptions isn't desirable.
@shawncal shawncal force-pushed the validationexception branch from 83ba112 to a9666de Compare May 1, 2023 17:00
adrianwyatt
adrianwyatt previously approved these changes May 1, 2023
@adrianwyatt adrianwyatt self-assigned this May 1, 2023
@adrianwyatt adrianwyatt requested review from dluc and shawncal May 1, 2023 17:46
@adrianwyatt adrianwyatt added the PR: ready for review All feedback addressed, ready for reviews label May 1, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@shawncal shawncal merged commit ac12a1a into microsoft:main May 1, 2023
awharrison-28 pushed a commit to awharrison-28/semantic-kernel that referenced this pull request May 1, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Remove custom ValidationException in favor of using built-in .NET
exception types typically used for the same purposes.

### Description
The conditions ValidationException was used for validating are better
suited to existing .NET exception types that .NET developers expect in
those situations.

- NotNullOrWhiteSpace is represented with ArgumentException.
- Directory not found is represented with DirectoryNotFoundException
- Invalid state of an object (e.g. a field not being initialized) is
represented with InvalidOperationException

Argument validation exceptions are also usage exceptions typically not
intended to be caught. The same would go for the ValidationExceptions
that were serving the same purpose, so having a separate type identity
for those exceptions isn't desirable.
codebrain pushed a commit to searchpioneer/semantic-kernel that referenced this pull request May 16, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Remove custom ValidationException in favor of using built-in .NET
exception types typically used for the same purposes.

### Description
The conditions ValidationException was used for validating are better
suited to existing .NET exception types that .NET developers expect in
those situations.

- NotNullOrWhiteSpace is represented with ArgumentException.
- Directory not found is represented with DirectoryNotFoundException
- Invalid state of an object (e.g. a field not being initialized) is
represented with InvalidOperationException

Argument validation exceptions are also usage exceptions typically not
intended to be caught. The same would go for the ValidationExceptions
that were serving the same purpose, so having a separate type identity
for those exceptions isn't desirable.
dehoward pushed a commit to lemillermicrosoft/semantic-kernel that referenced this pull request Jun 1, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Remove custom ValidationException in favor of using built-in .NET
exception types typically used for the same purposes.

### Description
The conditions ValidationException was used for validating are better
suited to existing .NET exception types that .NET developers expect in
those situations.

- NotNullOrWhiteSpace is represented with ArgumentException.
- Directory not found is represented with DirectoryNotFoundException
- Invalid state of an object (e.g. a field not being initialized) is
represented with InvalidOperationException

Argument validation exceptions are also usage exceptions typically not
intended to be caught. The same would go for the ValidationExceptions
that were serving the same purpose, so having a separate type identity
for those exceptions isn't desirable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kernel Issues or pull requests impacting the core kernel .NET Issue or Pull requests regarding .NET code PR: ready for review All feedback addressed, ready for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants