-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add SolutionArgumentException with error types #74
base: main
Are you sure you want to change the base?
Conversation
@microsoft-github-policy-service agree company="Microsoft" |
src/Microsoft.VisualStudio.SolutionPersistence/Model/SolutionArgumentException.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.VisualStudio.SolutionPersistence/Model/SolutionArgumentException.cs
Outdated
Show resolved
Hide resolved
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.
after this is merged and published need to update source build dotnet/source-build-externals#392 then publish to dotnet9 and 10 feeds, update msbuild and then sdk. (that's one of many reasons i was suggesting to make your life and the sdk implementation simpler and handle this error cases less pedantically)
src/Microsoft.VisualStudio.SolutionPersistence/Model/SolutionArgumentException.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.VisualStudio.SolutionPersistence/Model/SolutionArgumentException.cs
Outdated
Show resolved
Hide resolved
…rgumentException.cs Co-authored-by: kasperk81 <[email protected]>
src/Microsoft.VisualStudio.SolutionPersistence/Model/SolutionArgumentException.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.VisualStudio.SolutionPersistence/Model/SolutionArgumentException.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.VisualStudio.SolutionPersistence/Model/SolutionArgumentException.cs
Outdated
Show resolved
Hide resolved
|
||
public class SolutionArgumentException : ArgumentException | ||
{ | ||
public SolutionErrorType Type; |
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.
readonly property?
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 agree, this should be a readonly property.
Currently it is difficult for users of this library to compare and classify
ArgumentException
s, especially since the messages are formatted, localized strings.This creates a new class
SolutionArgumentException
that additionally has aType
property that corresponds to a member of enumSolutionArgumentExceptionType
.