-
Notifications
You must be signed in to change notification settings - Fork 241
Include the thrown exception in CertificateChangeEventArg #3428
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
Include the thrown exception in CertificateChangeEventArg #3428
Conversation
|
@microsoft-github-policy-service agree company="Microsoft" |
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
@tlupes
but you have merge conflicts (public API files)
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
Thanks @tlupes
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.
changes LGTM, are there any tests to add at this level?
There's no clear test that this can be added to, and as this is simply setting a property, I'm not sure of the value of creating a new test just for this. |
Include the thrown exception in CertificateChangeEventArg
Add in the thrown exception into CertificateChangeEventArg so that subscriber can know why the change was triggered (in the case of deselection)
Description
We intend to consume this information to emit detailed telemetry about certificate usage failures.
#3427